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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to