[PATCH] D115942: [X86][MS] Change the alignment of f80 to 16 bytes on Windows 32bits to match with ICC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.


================
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();
----------------
rnk wrote:
> 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()) {
>    ....
> ```
> 
That's better. Thanks for the suggestion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115942/new/

https://reviews.llvm.org/D115942