-
Notifications
You must be signed in to change notification settings - Fork 273
Move and cleanup declarations of C compiler intrinsics #6707
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
Move and cleanup declarations of C compiler intrinsics #6707
Conversation
d45b127
to
388f47d
Compare
063e58f
to
964b863
Compare
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.
Sorry, question here because I cannot put it into a comment on the code (doesn't seem appropriate).
This appears to be adding a python script to print the compiler intrinsics from clang. It also mentions (on one of the commit messages) that this is supposed to substitute for an older script, but I don't see the older one being deleted anywhere.
Is this a mistake, or are both supposed to be used? Or is my understanding of the situation wrong?
My apologies, I'll try to clarify the commit message: it's not really a substitute as the existing (bash) script fetches declarations from GCC's repository, while the new Python script fetches Clang's declarations. These declarations overlap to a great extent, but aren't completely the same. That said, however, the GCC script is rather brittle. Perhaps removing that script actually is the right thing to do. For now I'll just amend the commit message, but we might consider removing the bash script. |
964b863
to
f0681d6
Compare
Codecov ReportBase: 78.34% // Head: 78.34% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #6707 +/- ##
========================================
Coverage 78.34% 78.34%
========================================
Files 1644 1645 +1
Lines 190313 190358 +45
========================================
+ Hits 149097 149133 +36
- Misses 41216 41225 +9
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
f0681d6
to
2935c26
Compare
2935c26
to
3b8a786
Compare
This is to make them easier to distinguish from any other header files: the declarations located in these files may need to be updated from time to time. The frequency of those updates, however, depends on the release cadence by other compilers. It is unrelated to our release cadence.
Thus far, we constructed most of the declarations using the get-gcc-builtins.sh script. We may need to continue to do so for GCC. Clang's declarations, however, seem easier to parse. This new script now takes care of this. In future, we may decide to completely remove the GCC-specific script, and build upon Clang's declarations with selective tweaks instead.
We did not consistently use (void) as the parameter type declaration.
To simplify textual comparison of declarations generated by clang_builtins.py and the existing ones, add "int" to types "unsigned," "long long," etc.
3b8a786
to
371d886
Compare
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 approving with the following caveats:
- Adding a new script seems philosophically dangerous, now we have multiple places to gather the data we need
- I don't think any of this is (well?) tested...
- While it looks ok to me, I'm not really an expect in the compiler intrinsics area
Points taken. Maybe we can one day try to come up with a better story for compiler intrinsics. At the moment, this certainly is just a best-effort and not very organised activity. |
Please review commit-by-commit: the main added value is a new script that fetches Clang's declarations. The remaining commits are cleanup to facilitate easier updates of these intrinsics in future.