This revision was automatically updated to reflect the committed changes.
Closed by commit rL247451: Record function attribute "stackrealign" instead of
using backend option (authored by ahatanak).
Changed prior to commit:
http://reviews.llvm.org/D11815?vs=33456&id=34569#toc
Repository:
rL L
ahatanak added a comment.
OK, thanks. I'll go ahead and commit this patch and the llvm-side patch.
http://reviews.llvm.org/D11815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
In http://reviews.llvm.org/D11815#243031, @hfinkel wrote:
> In http://reviews.llvm.org/D11815#242616, @ahatanak wrote:
>
> > Hal, do you have any thoughts on the points Vasileios brought up?
hfinkel added a comment.
In http://reviews.llvm.org/D11815#242616, @ahatanak wrote:
> Hal, do you have any thoughts on the points Vasileios brought up? Currently,
> many of the targets don't guarantee that the realigned stack is at least as
> aligned as the default alignment required by the ABI
ahatanak added a comment.
Hal, do you have any thoughts on the points Vasileios brought up? Currently,
many of the targets don't guarantee that the realigned stack is at least as
aligned as the default alignment required by the ABI. Is this the behavior
end-users expect when they use -mstackrea
ahatanak added a comment.
In http://reviews.llvm.org/D11815#238209, @vkalintiris wrote:
> In http://reviews.llvm.org/D11815#237541, @ahatanak wrote:
>
> > In http://reviews.llvm.org/D11815#236368, @vkalintiris wrote:
> >
> > > In http://reviews.llvm.org/D11815#235394, @ahatanak wrote:
> > >
> > >
vkalintiris added a comment.
In http://reviews.llvm.org/D11815#237541, @ahatanak wrote:
> In http://reviews.llvm.org/D11815#236368, @vkalintiris wrote:
>
> > In http://reviews.llvm.org/D11815#235394, @ahatanak wrote:
> >
> > >
> >
>
>
>
>
> > For example, on a Mips target, where the O32 ABI requi
ahatanak added a comment.
In http://reviews.llvm.org/D11815#236368, @vkalintiris wrote:
> In http://reviews.llvm.org/D11815#235394, @ahatanak wrote:
>
> >
>
> For example, on a Mips target, where the O32 ABI requires either way an
> 8-byte alignment, we would generate redundant code for real
ahatanak added a comment.
In http://reviews.llvm.org/D11815#236368, @vkalintiris wrote:
> In http://reviews.llvm.org/D11815#235394, @ahatanak wrote:
>
> > The cc1 option "-mstackrealign" now means "force stack realignment" rather
> > than "allow stack realignment" and causes function attribute "
vkalintiris added a subscriber: vkalintiris.
vkalintiris added a comment.
In http://reviews.llvm.org/D11815#235394, @ahatanak wrote:
> The cc1 option "-mstackrealign" now means "force stack realignment" rather
> than "allow stack realignment" and causes function attribute "stackrealign"
> to be
ahatanak updated this revision to Diff 33456.
ahatanak added a comment.
This updated patch changes the handling of driver option
-mstackrealign/-mno-stackrealign. -mno-stackrealign no longer indicates stack
realignment should be disallowed and clang no longer attaches function
attribute "no-rea
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:
> > > > ec
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:
> > > > hf
ahatanak added inline comments.
Comment at: lib/Driver/Tools.cpp:4232
@@ +4231,3 @@
+ false))
+CmdArgs.push_back(Args.MakeArgString("-force-align-stack"));
+
echristo wrote:
> hfinkel wrote:
> > echristo wrote:
> > > hfinkel wrote:
> > > > Th
echristo added inline comments.
Comment at: lib/Driver/Tools.cpp:4232
@@ +4231,3 @@
+ false))
+CmdArgs.push_back(Args.MakeArgString("-force-align-stack"));
+
hfinkel wrote:
> echristo wrote:
> > hfinkel wrote:
> > > The code below for OPT_mst
hfinkel added inline comments.
Comment at: lib/Driver/Tools.cpp:4232
@@ +4231,3 @@
+ false))
+CmdArgs.push_back(Args.MakeArgString("-force-align-stack"));
+
echristo wrote:
> hfinkel wrote:
> > The code below for OPT_mstackrealign uses -mstac
echristo added inline comments.
Comment at: lib/Driver/Tools.cpp:4232
@@ +4231,3 @@
+ false))
+CmdArgs.push_back(Args.MakeArgString("-force-align-stack"));
+
hfinkel wrote:
> The code below for OPT_mstackrealign uses -mstackrealign as the nam
hfinkel added inline comments.
Comment at: lib/Driver/Tools.cpp:4232
@@ +4231,3 @@
+ false))
+CmdArgs.push_back(Args.MakeArgString("-force-align-stack"));
+
The code below for OPT_mstackrealign uses -mstackrealign as the name of the
backend
ahatanak updated this revision to Diff 32592.
ahatanak added a comment.
This patch makes changes to record function attribute "force-align-stack"
instead of recording it as a subtarget feature.
http://reviews.llvm.org/D11815
Files:
include/clang/Driver/CC1Options.td
include/clang/Frontend/
On Mon, Aug 17, 2015 at 12:41 PM, Eric Christopher
wrote:
>
>
> On Mon, Aug 17, 2015 at 11:57 AM Akira Hatanaka
> wrote:
>
>> On Thu, Aug 13, 2015 at 5:56 PM, Eric Christopher
>> wrote:
>>
>>>
> Apologies, I'm really resistant to more things being used in
> TargetOptions and I was (pe
On Mon, Aug 17, 2015 at 11:57 AM Akira Hatanaka wrote:
> On Thu, Aug 13, 2015 at 5:56 PM, Eric Christopher
> wrote:
>
>>
>>> > Apologies, I'm really resistant to more things being used in
>>> > TargetOptions and I was (perhaps mistakenly) under the impression
>>> > that you wanted to move it to
On Thu, Aug 13, 2015 at 5:56 PM, Eric Christopher
wrote:
>
>> > Apologies, I'm really resistant to more things being used in
>> > TargetOptions and I was (perhaps mistakenly) under the impression
>> > that you wanted to move it to TargetOptions without an IR
>> > serialization. We need all option
>
>
> > Apologies, I'm really resistant to more things being used in
> > TargetOptions and I was (perhaps mistakenly) under the impression
> > that you wanted to move it to TargetOptions without an IR
> > serialization. We need all options to have that sort of
> > serialization right? :)
>
> Absolu
fin...@anl.gov
> > Cc: cfe-commits@lists.llvm.org
> > Sent: Thursday, August 13, 2015 7:39:53 PM
> > Subject: Re: [PATCH] D11815: Pass subtarget feature "force-align-stack"
> >
> > On Thu, Aug 13, 2015 at 5:16 PM hfin...@anl.gov < hfin...@anl.gov >
&
bject: Re: [PATCH] D11815: Pass subtarget feature "force-align-stack"
>
> On Thu, Aug 13, 2015 at 5:16 PM hfin...@anl.gov < hfin...@anl.gov >
> wrote:
>
>
> hfinkel added a comment.
>
> In http://reviews.llvm.org/D11815#224169 , @echristo wrote:
>
> &
On Thu, Aug 13, 2015 at 5:16 PM hfin...@anl.gov wrote:
> hfinkel added a comment.
>
> In http://reviews.llvm.org/D11815#224169, @echristo wrote:
>
> > No, RESET_OPTION isn't the right way to do this either. For exactly this
> sort of reason. You can't actually represent all of the code and option
hfinkel added a comment.
In http://reviews.llvm.org/D11815#224169, @echristo wrote:
> No, RESET_OPTION isn't the right way to do this either. For exactly this sort
> of reason. You can't actually represent all of the code and options this way
> in the IR. If you can't do that then it's a non-st
echristo added a comment.
No, RESET_OPTION isn't the right way to do this either. For exactly this sort
of reason. You can't actually represent all of the code and options this way in
the IR. If you can't do that then it's a non-starter.
-eric
http://reviews.llvm.org/D11815
___
hfinkel added a comment.
In http://reviews.llvm.org/D11815#224161, @echristo wrote:
> Hi Hal,
>
> No, TargetOptions is exactly the wrong place to handle this due to wanting to
> have various functions compiled with and without a force aligned stack at the
> IR level that might not hold up at LT
echristo added a comment.
Hi Hal,
No, TargetOptions is exactly the wrong place to handle this due to wanting to
have various functions compiled with and without a force aligned stack at the
IR level that might not hold up at LTO time.
-eric
http://reviews.llvm.org/D11815
_
hfinkel added a subscriber: hfinkel.
hfinkel requested changes to this revision.
hfinkel added a reviewer: hfinkel.
hfinkel added a comment.
This revision now requires changes to proceed.
As I've said in the review for http://reviews.llvm.org/D11814, this should be
added to TargetOptions, and con
ahatanak created this revision.
ahatanak added reviewers: echristo, dexonsmith.
ahatanak added a subscriber: cfe-commits.
This patch makes changes to pass subtarget feature "force-align-stack" instead
of passing a backend-option when users provide "-mstackrealign" on the command
line.
The llvm-
32 matches
Mail list logo
