[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 subscribers: jyknight, thakis.
rnk added a comment.
This revision is now accepted and ready to land.

Thanks, looks good. I have a minor code refactoring suggestion, but feel free to resolve it as you see fit and land it.



================
Comment at: clang/lib/Basic/Targets/X86.h:534-535
     DoubleAlign = LongLongAlign = 64;
     bool IsWinCOFF =
         getTriple().isOSWindows() && getTriple().isOSBinFormatCOFF();
     resetDataLayout(IsWinCOFF ? "e-m:x-p:32:32-p270:32:32-p271:32:32-p272:64:"
----------------
I think it would be simpler to make the MSVC mode data layout change here.

It is also my understanding that, prior to rGb214cbc7852fa1719c5d0b, only long-lived string constants could be passed to `resetDataLayout`. Since 2016, that is unneeded. `resetDataLayout` now copies its string argument. (@jyknight @thakis, who made the changes here).

I suggest structuring the code like this:

```
bool IsWinCOFF = getTriple()...
bool IsMSVC = getTriple()...
StringRef Mangling = IsWinCOFF ? "m:x" : "m:e";
StringRef F80 = IsMSVC ? "f80:128" : "f80:32";
std::string Layout = "e-" + Mangling + "-p:32..." + F80 + "-n8:...";
resetDataLayout(Layout, IsWinCOFF ? "_" : "");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115942