hfinkel added inline comments.

================
Comment at: lib/Driver/Tools.cpp:4232
@@ +4231,3 @@
+                   false))
+    CmdArgs.push_back(Args.MakeArgString("-force-align-stack"));
+
----------------
ahatanak wrote:
> echristo wrote:
> > hfinkel wrote:
> > > echristo wrote:
> > > > hfinkel wrote:
> > > > > The code below for OPT_mstackrealign uses -mstackrealign as the name 
> > > > > of the backend option too. Why not do the same for OPT_mstackrealign 
> > > > > (use -mstackrealign as the name of the backend option) instead of 
> > > > > inventing a new flag name -force-align-stack?
> > > > In general we don't do that. But I also don't want this to use a 
> > > > backend option anyhow, why are we doing that here once we have the 
> > > > attribute?
> > > It's not a backend option, this is the flag being passed from the driver 
> > > to clang -cc1.
> > > 
> > Aha. I must have misread. Then I totally agree :)
> I believe the confusing part here is that the CC1 option "-mstackrealign" in 
> the code below indicates stack realignment is allowed, but not necessarily 
> forced (see the definition of StackRealignment in CodeGenOptions.def). This 
> is different from the driver option options::OPT_mstackrealign, which 
> indicates stack alignment should be forced.
> 
> Does that answer your question?
Yes, but this somehow makes things seem even more broken. As I understand it, 
we have two underlying CodeGen options here:

 1. May the backend realign the stack if it thinks that it should? [Mainly 
because it has some overaligned local variable to put on the stack]. This is on 
by default.
 2. Must the backend realign all functions. This is off by default.

GCC has an option -mstackrealign documented to mean only (2). But we currently 
use its inverse (-mno-stackrealign) to mean the inverse of (1). Frankly, (1) 
seems like a debugging option, and I don't see why we are exposing it to users. 
If we have overaligned locals, than the backend should realign the stack. 
Always. Otherwise the code is broken. Then we can use -mstackrealign for its 
intended purpose of only meaning (2), and -mno-stackrealign the inverse of (2).



http://reviews.llvm.org/D11815



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

Reply via email to