[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]

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