ahatanak added inline comments.
================
Comment at: lib/Driver/Tools.cpp:4232
@@ +4231,3 @@
+ false))
+ CmdArgs.push_back(Args.MakeArgString("-force-align-stack"));
+
----------------
hfinkel wrote:
> 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).
>
I think you are right. gcc's -mno-stackrealign doesn't disallow stack
realignment. It cancels a -mstackrealign preceding it, but has no effect if it
appears alone on the command line.
I'll upload a new patch shortly.
http://reviews.llvm.org/D11815
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
