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

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

Reply via email to