Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preparations to make RIOT 64-bit ready #20154
Preparations to make RIOT 64-bit ready #20154
Changes from 1 commit
8e604fe
4067cc3
adf612f
6204ef6
7eca6b0
9058c6d
dff93a4
a9dd66b
9a4549b
2157de4
c175f02
e3f34ef
c4181d6
6081fb2
71e49c5
464f511
a6e7cca
47ce2a2
60a2241
e0889fb
3d02eb6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that one I don't understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question - @fzi-haxel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On x86_64,
__SIZEOF_INT128__
is defined, therefore__int128
is used, but this leads to this GCC warning:In retrospect, simply not using
__int128
does not seem to be the most elegant solution. Another option would be:However, the GCC documentation points out the following when using
__extension__
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused: clang accepts
__int128
even though it is not part of the C standard but gcc does not?What I also do not understand: if
__int128
is a built-in data type on gcc (>= 4.1) wouldn't I still get the warning even with the typedef? (Which definitiv would be actually used?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
-Wpedantic
is enabled, yes.Example: https://godbolt.org/z/bsYq49jGY
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
__extension__
would get rid of the warning.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, actually, I tried this (using gcc 13.2.1) but still got the warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure? I just tested it with (13.2.0) and didn't get this warning.
But GCC 13 now gives this warning during linking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, no, sorry, I think there was a PEBCAK issue (I always confuse the order of arguments for typedef).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened an issue upstream with some suggested solutions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are using our own fork anyway, you might as well open the PR there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True - but actually unrelated to this PR, isn't it?Sorry - got confused by the highlighting for the patch file here. I thought that was an actual diff instead of an added file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a PR in the fork.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR is merged, waiting for #20208 to get merged before we can drop this patch.