[PATCH] D115942: [X86][MS] Change the alignment of f80 to 16 bytes on Windows 32bits to match with ICC
Reid Kleckner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 11 12:17:41 PST 2022
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
I'm happy with this, but I'd like to get an ack from @craig.topper or @efriedma about the backwards compatibility concern. I think it's addresed.
================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:4583-4589
+ if (T.isArch64Bit() || !T.isWindowsMSVCEnvironment())
+ return Res;
- return (Groups[1] + AddrSpaces + Groups[3]).str();
+ StringRef Ref = Res;
+ auto I = Ref.find("-f80:32-");
+ if (I != StringRef::npos)
+ Res = (Ref.take_front(I) + "-f80:128-" + Ref.drop_front(I + 8)).str();
----------------
I think the early return here is not helping readability. Please add a comment here about why this upgrade is being done, something like:
```
// For 32-bit MSVC targets, raise the alignment of f80 values to 16 bytes. Raising the alignment is safe because Clang did not produce f80 values in the MSVC environment before this upgrade was added.
if (T.isWindowsMSVCEnvironment() && T.isArch32Bit()) {
....
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115942/new/
https://reviews.llvm.org/D115942
More information about the llvm-commits
mailing list