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