The ACLE states that 64-bit crc32, wsr, rsr and rbit operands are uint64_t so we should have the clang builtin match this description - which is what we already do for AArch32.
Details
- Reviewers
t.p.northover efriedma phosek rnk - Commits
- rGf66fbcfb52f5: Merging r352463: --------------------------------------------------------------…
rG015f97db8b34: [AArch64] Update int64_t ACLE builtin arguments
rGa96f8461e715: [AArch64] Use LL for 64-bit intrinsic arguments
rL352860: Merging r352463:
rC352463: [AArch64] Update int64_t ACLE builtin arguments
rL352463: [AArch64] Update int64_t ACLE builtin arguments
rC351740: [AArch64] Use LL for 64-bit intrinsic arguments
rL351740: [AArch64] Use LL for 64-bit intrinsic arguments
Diff Detail
Event Timeline
include/clang/Basic/BuiltinsAArch64.def | ||
---|---|---|
53 | If we can't change the signature on Linux (I don't see any reason why couldn't, though) we can use the 'W' builtin code to indicate that it's long on LP64 and long long on LLP64. |
While you're here, can you also fix __builtin_arm_rbit64, __builtin_arm_rsr64, and __builtin_arm_wsr64?
include/clang/Basic/BuiltinsAArch64.def | ||
---|---|---|
53 | Changing the signature would be fine, I think (we don't expect people to use these directly, and even if they did it wouldn't make any practical difference outside of obscure edge cases). But probably better to match the ACLE signature for the sake of clarity. |
This broke our kernel build which uses PRIx64 to print the return value of __builtin_arm_rsr64. This is correct because according to ACLE, the return type of that builtin should be uint64_t. Problem is that on AArch64, unsigned long is equivalent to uint64_t, not unsigned long long (at least on Linux and other platforms like Fuchsia).
The reason this wasn't caught by the test is because that test isn't strict enough. I've updated the test in D57210. I believe this change should be reverted because it's wrong and I created revert in D57209. I think the correct solution might be what rnk suggested in his comment, that is to use W instead of LL. I plan on landing my revert if I don't hear back by EOD because this is currently blocking our kernel build with the latest Clang.
Changed the builtins to use W instead of LL. I've also updated the tests, adding a test for rbitl.
If we can't change the signature on Linux (I don't see any reason why couldn't, though) we can use the 'W' builtin code to indicate that it's long on LP64 and long long on LLP64.