[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:6067-6070
+    // Alignment calculations can wrap around if it's greater than 2**28.
+    unsigned MaximumAlignment =
+        Context.getTargetInfo().getTriple().isOSBinFormatCOFF() ? 8192
+                                                                : 268435456;
----------------
erichkeane wrote:
> rsmith wrote:
> > Why is there a different limit depending on bin format? We can support this at the IR level regardless, can't we? (I don't see how the binary format is relevant.)
> I'd copied it from the Sema::AddAlignedAttr implementation, but I cannot seem to figure out the origin of that.  @majnemer added the 2**28 business back in 2015, but @aaron.ballman put the limit of 8192 in here: https://reviews.llvm.org/rL158717#change-N0HH8qtBJv7d
> (note it was reverted and relanded).
> 
> I don't see sufficient justification in that history now that I've looked back to keep that log in here, so I'll keep us at 2**28.
> 
It's been a while, but I seem to recall this matching a behavior of MSVC at the time. 


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

https://reviews.llvm.org/D68824