| Message ID | CAJK_mQ1NB7gfroQuAi+H_+TXzb51KPaMK2EsEoFFswvTL4fCFQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote: > Hi maintainers, > > I just added "=r" and retested it. I had a very similar patch to this sitting in my local tree. However, I am surprised you have left operand 3 as an output operand. In my tree I had marked operand 3 as "&r". What do you think? Thanks, James > gcc/ChangeLog > > 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> > > * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add register > constraint for operand 0. > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index b5be79c..ed6e602 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -4026,7 +4026,7 @@ > }) > > (define_insn "stack_protect_test_<mode>" > - [(set (match_operand:PTR 0 "register_operand") > + [(set (match_operand:PTR 0 "register_operand" "=r") > (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") > (match_operand:PTR 2 "memory_operand" "m")] > UNSPEC_SP_TEST)) > > regards, > venkat. > > On 4 September 2014 12:42, Venkataramanan Kumar > <venkataramanan.kumar@linaro.org> wrote: > > Hi Maintainers, > > > > Below patch adds register constraint "r" for destination operand in > > "stack_protect_test" pattern. > > > > We need a general register here and adding "r" will avoid vector > > register getting allocated. > > > > regression tested on aarch64-unknown-linux-gnu. > > > > Ok for trunk? > > > > regards, > > Venkat. > > > > > > gcc/ChangeLog > > > > 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> > > > > * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add register > > constraint for operand 0. > > > > > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > > index b5be79c..77588b9 100644 > > --- a/gcc/config/aarch64/aarch64.md > > +++ b/gcc/config/aarch64/aarch64.md > > @@ -4026,7 +4026,7 @@ > > }) > > > > (define_insn "stack_protect_test_<mode>" > > - [(set (match_operand:PTR 0 "register_operand") > > + [(set (match_operand:PTR 0 "register_operand" "r") > > (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") > > (match_operand:PTR 2 "memory_operand" "m")] > > UNSPEC_SP_TEST)) >
On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh <james.greenhalgh@arm.com> wrote: > On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote: >> Hi maintainers, >> >> I just added "=r" and retested it. > > I had a very similar patch to this sitting in my local tree. However, > I am surprised you have left operand 3 as an output operand. In my tree > I had marked operand 3 as "&r". > > What do you think? The clobber needs to be "=&r" as you are writing to the register and not just reading from it. I think this is causing some issues including linaro bugzilla #667 (https://bugs.linaro.org/show_bug.cgi?id=667). Thanks, Andrew Pinski > > Thanks, > James > >> gcc/ChangeLog >> >> 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> >> >> * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add register >> constraint for operand 0. >> >> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md >> index b5be79c..ed6e602 100644 >> --- a/gcc/config/aarch64/aarch64.md >> +++ b/gcc/config/aarch64/aarch64.md >> @@ -4026,7 +4026,7 @@ >> }) >> >> (define_insn "stack_protect_test_<mode>" >> - [(set (match_operand:PTR 0 "register_operand") >> + [(set (match_operand:PTR 0 "register_operand" "=r") >> (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") >> (match_operand:PTR 2 "memory_operand" "m")] >> UNSPEC_SP_TEST)) >> >> regards, >> venkat. >> >> On 4 September 2014 12:42, Venkataramanan Kumar >> <venkataramanan.kumar@linaro.org> wrote: >> > Hi Maintainers, >> > >> > Below patch adds register constraint "r" for destination operand in >> > "stack_protect_test" pattern. >> > >> > We need a general register here and adding "r" will avoid vector >> > register getting allocated. >> > >> > regression tested on aarch64-unknown-linux-gnu. >> > >> > Ok for trunk? >> > >> > regards, >> > Venkat. >> > >> > >> > gcc/ChangeLog >> > >> > 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> >> > >> > * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add register >> > constraint for operand 0. >> > >> > >> > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md >> > index b5be79c..77588b9 100644 >> > --- a/gcc/config/aarch64/aarch64.md >> > +++ b/gcc/config/aarch64/aarch64.md >> > @@ -4026,7 +4026,7 @@ >> > }) >> > >> > (define_insn "stack_protect_test_<mode>" >> > - [(set (match_operand:PTR 0 "register_operand") >> > + [(set (match_operand:PTR 0 "register_operand" "r") >> > (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") >> > (match_operand:PTR 2 "memory_operand" "m")] >> > UNSPEC_SP_TEST)) >>
Hi Andrew, Thanks for pointing that. I thought "&" modifier is enough to say that operand is early clobbered and so GCC will use a different register and it will not allocate same register that was given to a input operand. Lookign at the the bug it looks like "=" is needed for the clobber, so that GCC will allocate a fresh register. regards, Venkat. On 17 September 2014 03:06, Andrew Pinski <pinskia@gmail.com> wrote: > On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh > <james.greenhalgh@arm.com> wrote: > > On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote: > >> Hi maintainers, > >> > >> I just added "=r" and retested it. > > > > I had a very similar patch to this sitting in my local tree. However, > > I am surprised you have left operand 3 as an output operand. In my tree > > I had marked operand 3 as "&r". > > > > What do you think? > > The clobber needs to be "=&r" as you are writing to the register and > not just reading from it. I think this is causing some issues > including linaro bugzilla #667 > (https://bugs.linaro.org/show_bug.cgi?id=667). > > Thanks, > Andrew Pinski > > > > > > Thanks, > > James > > > >> gcc/ChangeLog > >> > >> 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> > >> > >> * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add > register > >> constraint for operand 0. > >> > >> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/ > aarch64.md > >> index b5be79c..ed6e602 100644 > >> --- a/gcc/config/aarch64/aarch64.md > >> +++ b/gcc/config/aarch64/aarch64.md > >> @@ -4026,7 +4026,7 @@ > >> }) > >> > >> (define_insn "stack_protect_test_<mode>" > >> - [(set (match_operand:PTR 0 "register_operand") > >> + [(set (match_operand:PTR 0 "register_operand" "=r") > >> (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") > >> (match_operand:PTR 2 "memory_operand" "m")] > >> UNSPEC_SP_TEST)) > >> > >> regards, > >> venkat. > >> > >> On 4 September 2014 12:42, Venkataramanan Kumar > >> <venkataramanan.kumar@linaro.org> wrote: > >> > Hi Maintainers, > >> > > >> > Below patch adds register constraint "r" for destination operand in > >> > "stack_protect_test" pattern. > >> > > >> > We need a general register here and adding "r" will avoid vector > >> > register getting allocated. > >> > > >> > regression tested on aarch64-unknown-linux-gnu. > >> > > >> > Ok for trunk? > >> > > >> > regards, > >> > Venkat. > >> > > >> > > >> > gcc/ChangeLog > >> > > >> > 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> > >> > > >> > * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add > register > >> > constraint for operand 0. > >> > > >> > > >> > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/ > aarch64.md > >> > index b5be79c..77588b9 100644 > >> > --- a/gcc/config/aarch64/aarch64.md > >> > +++ b/gcc/config/aarch64/aarch64.md > >> > @@ -4026,7 +4026,7 @@ > >> > }) > >> > > >> > (define_insn "stack_protect_test_<mode>" > >> > - [(set (match_operand:PTR 0 "register_operand") > >> > + [(set (match_operand:PTR 0 "register_operand" "r") > >> > (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") > >> > (match_operand:PTR 2 "memory_operand" "m")] > >> > UNSPEC_SP_TEST)) > >> >
Hi Andrew, Thanks for pointing that. I thought "&" modifier is enough to say that operand is early clobbered and so GCC will use a different register and it will not allocate same register that was given to a input operand. Lookign at the the bug it looks like "=" is needed for the clobber, so that GCC will allocate a fresh register. regards, Venkat. On 17 September 2014 03:06, Andrew Pinski <pinskia@gmail.com> wrote: > On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh > <james.greenhalgh@arm.com> wrote: >> On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote: >>> Hi maintainers, >>> >>> I just added "=r" and retested it. >> >> I had a very similar patch to this sitting in my local tree. However, >> I am surprised you have left operand 3 as an output operand. In my tree >> I had marked operand 3 as "&r". >> >> What do you think? > > The clobber needs to be "=&r" as you are writing to the register and > not just reading from it. I think this is causing some issues > including linaro bugzilla #667 > (https://bugs.linaro.org/show_bug.cgi?id=667). > > Thanks, > Andrew Pinski > > >> >> Thanks, >> James >> >>> gcc/ChangeLog >>> >>> 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> >>> >>> * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add register >>> constraint for operand 0. >>> >>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md >>> index b5be79c..ed6e602 100644 >>> --- a/gcc/config/aarch64/aarch64.md >>> +++ b/gcc/config/aarch64/aarch64.md >>> @@ -4026,7 +4026,7 @@ >>> }) >>> >>> (define_insn "stack_protect_test_<mode>" >>> - [(set (match_operand:PTR 0 "register_operand") >>> + [(set (match_operand:PTR 0 "register_operand" "=r") >>> (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") >>> (match_operand:PTR 2 "memory_operand" "m")] >>> UNSPEC_SP_TEST)) >>> >>> regards, >>> venkat. >>> >>> On 4 September 2014 12:42, Venkataramanan Kumar >>> <venkataramanan.kumar@linaro.org> wrote: >>> > Hi Maintainers, >>> > >>> > Below patch adds register constraint "r" for destination operand in >>> > "stack_protect_test" pattern. >>> > >>> > We need a general register here and adding "r" will avoid vector >>> > register getting allocated. >>> > >>> > regression tested on aarch64-unknown-linux-gnu. >>> > >>> > Ok for trunk? >>> > >>> > regards, >>> > Venkat. >>> > >>> > >>> > gcc/ChangeLog >>> > >>> > 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> >>> > >>> > * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add register >>> > constraint for operand 0. >>> > >>> > >>> > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md >>> > index b5be79c..77588b9 100644 >>> > --- a/gcc/config/aarch64/aarch64.md >>> > +++ b/gcc/config/aarch64/aarch64.md >>> > @@ -4026,7 +4026,7 @@ >>> > }) >>> > >>> > (define_insn "stack_protect_test_<mode>" >>> > - [(set (match_operand:PTR 0 "register_operand") >>> > + [(set (match_operand:PTR 0 "register_operand" "r") >>> > (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") >>> > (match_operand:PTR 2 "memory_operand" "m")] >>> > UNSPEC_SP_TEST)) >>>
On Tue, Sep 16, 2014 at 10:36:08PM +0100, Andrew Pinski wrote: > On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh > <james.greenhalgh@arm.com> wrote: > > On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote: > >> Hi maintainers, > >> > >> I just added "=r" and retested it. > > > > I had a very similar patch to this sitting in my local tree. However, > > I am surprised you have left operand 3 as an output operand. In my tree > > I had marked operand 3 as "&r". > > > > What do you think? > > The clobber needs to be "=&r" as you are writing to the register and > not just reading from it. I think this is causing some issues > including linaro bugzilla #667 > (https://bugs.linaro.org/show_bug.cgi?id=667). (+CC Matthias Klose and Steve McIntyre who have also been in contact with me regarding this bug) I've seen this bug locally, and had considered sending the patch you suggested, which does indeed fix the bug. However, it feels wrong as the operand is not a formal output of the pattern. It is clobbered - and indeed earlyclobbered - so yes it is written to, but it isn't an output. This makes the fix look like a band-aid around the real problem. The bug looks similar to pr52573 - regrename fails to spot that it should not rename to a register used in an earlyclobber operand of any type, rather than just an output+earlyclobber operand as it does now. I've played about with a fix that sits in regrename, and forces it to think of all earlyclobber operands as starting and ending chains but this didn't bootstrap clean - we end up with what I believe are false reports of stack smashing in libstdc++. I was planning to look again at my approach tomorrow, I would like to convince myself that this isn't a deficiency in regrename before I would support just marking this operand "=&r". If you have any other suggestions, or if "=&r" is actually correct and I am misreading the documentation please let me know. Thanks, James
On Tue, Sep 16, 2014 at 3:03 PM, James Greenhalgh <james.greenhalgh@arm.com> wrote: > On Tue, Sep 16, 2014 at 10:36:08PM +0100, Andrew Pinski wrote: >> On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh >> <james.greenhalgh@arm.com> wrote: >> > On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote: >> >> Hi maintainers, >> >> >> >> I just added "=r" and retested it. >> > >> > I had a very similar patch to this sitting in my local tree. However, >> > I am surprised you have left operand 3 as an output operand. In my tree >> > I had marked operand 3 as "&r". >> > >> > What do you think? >> >> The clobber needs to be "=&r" as you are writing to the register and >> not just reading from it. I think this is causing some issues >> including linaro bugzilla #667 >> (https://bugs.linaro.org/show_bug.cgi?id=667). > > (+CC Matthias Klose and Steve McIntyre who have also been in contact with me > regarding this bug) > > I've seen this bug locally, and had considered sending the patch you > suggested, which does indeed fix the bug. However, it feels wrong as > the operand is not a formal output of the pattern. It is clobbered - and > indeed earlyclobbered - so yes it is written to, but it isn't an output. > This makes the fix look like a band-aid around the real problem. > > The bug looks similar to pr52573 - regrename fails to spot that it should > not rename to a register used in an earlyclobber operand of any type, rather > than just an output+earlyclobber operand as it does now. > > I've played about with a fix that sits in regrename, and forces it to think > of all earlyclobber operands as starting and ending chains but this didn't > bootstrap clean - we end up with what I believe are false reports of stack > smashing in libstdc++. > > I was planning to look again at my approach tomorrow, I would like to > convince myself that this isn't a deficiency in regrename before I would > support just marking this operand "=&r". > > If you have any other suggestions, or if "=&r" is actually correct and > I am misreading the documentation please let me know. I think you misread the documentation. Or rather the documentation is not fully clear here. https://gcc.gnu.org/onlinedocs/gccint/Modifiers.html says this for "&": ‘&’ does not obviate the need to write ‘=’ or ‘+’. Which means you need "=" if you write to the register. Match_scratch is the same as match_operand except it is able to be added back during combine. Also "=" means: Means that this operand is write-only for this instruction: the previous value is discarded and replaced by output data. That is not necessary a formal output of the pattern. Thanks, Andrew Thanks, Andrew Pinski > > Thanks, > James >
On 16/09/14 23:03, James Greenhalgh wrote: > On Tue, Sep 16, 2014 at 10:36:08PM +0100, Andrew Pinski wrote: >> On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh >> <james.greenhalgh@arm.com> wrote: >>> On Thu, Sep 04, 2014 at 08:42:31AM +0100, Venkataramanan Kumar wrote: >>>> Hi maintainers, >>>> >>>> I just added "=r" and retested it. >>> >>> I had a very similar patch to this sitting in my local tree. However, >>> I am surprised you have left operand 3 as an output operand. In my tree >>> I had marked operand 3 as "&r". >>> >>> What do you think? >> >> The clobber needs to be "=&r" as you are writing to the register and >> not just reading from it. I think this is causing some issues >> including linaro bugzilla #667 >> (https://bugs.linaro.org/show_bug.cgi?id=667). > > (+CC Matthias Klose and Steve McIntyre who have also been in contact with me > regarding this bug) > > I've seen this bug locally, and had considered sending the patch you > suggested, which does indeed fix the bug. However, it feels wrong as > the operand is not a formal output of the pattern. It is clobbered - and > indeed earlyclobbered - so yes it is written to, but it isn't an output. > This makes the fix look like a band-aid around the real problem. > > The bug looks similar to pr52573 - regrename fails to spot that it should > not rename to a register used in an earlyclobber operand of any type, rather > than just an output+earlyclobber operand as it does now. > > I've played about with a fix that sits in regrename, and forces it to think > of all earlyclobber operands as starting and ending chains but this didn't > bootstrap clean - we end up with what I believe are false reports of stack > smashing in libstdc++. > > I was planning to look again at my approach tomorrow, I would like to > convince myself that this isn't a deficiency in regrename before I would > support just marking this operand "=&r". > > If you have any other suggestions, or if "=&r" is actually correct and > I am misreading the documentation please let me know. > > Thanks, > James > "=&r" is correct for an early-clobbered scratch. R.
> On Sep 17, 2014, at 7:43 AM, James Greenhalgh <james.greenhalgh@arm.com> wrote: > > >> On Wed, Sep 17, 2014 at 09:30:31AM +0100, Richard Earnshaw wrote: >> "=&r" is correct for an early-clobbered scratch. >> >> R. > > In that case... > > How is the attached patch for trunk? I've bootstrapped it on AArch64 > with -fstack-protector-strong and -frename-registers in the BOOT_CFLAGS > without seeing any issues. > > OK? > > Thanks, > James > > --- > gcc/ > > 2014-09-15 James Greenhalgh <james.greenhalgh@arm.com> > > * config/aarch64/aarch64.md (stack_protect_test_<mode>): Mark > scratch register as an output to placate register renaming. > > gcc/testsuite/ > > 2014-09-15 James Greenhalgh <james.greenhalgh@arm.com> > > * gcc.target/aarch64/stack_protector_set_1.c: New. > * gcc.target/aarch64/stack_protector_set_2.c: Likewise. There is nothing aarch64 specific about this testcase so I would place them under gcc.dg and add the extra marker which says this testcase requires stack protector. And maybe even use compile instead of just assemble too. Thanks, Andrew > <0001-Re-PATCH-AArch64-Add-constraint-letter-for-stack_pro.patch>
Am 17.09.2014 um 15:14 schrieb Matthias Klose: > Am 17.09.2014 um 00:03 schrieb James Greenhalgh: >> If you have any other suggestions, or if "=&r" is actually correct and >> I am misreading the documentation please let me know. > > with this patch I see a lot of ICEs in the testsuite for test cases built with > -O3 (and a build defaulting to -fstack-protector-strong by default), all of the > form: > > Executing on host: /home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/xgcc > -B/home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/ -fno-diagnostics-show-caret -fdia > gnostics-color=never -O3 -fomit-frame-pointer -funroll-all-loops > -finline-functions -w -c -o 900116-1.o /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/g > cc/testsuite/gcc.c-torture/compile/900116-1.c (timeout = 300) > spawn /home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/xgcc > -B/home/doko/gcc/4.9/gcc-4.9-4.9.1/build/gcc/ -fno-diagnostics-show-caret > -fdiagnostics-color > =never -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions -w -c -o > 900116-1.o /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c- > torture/compile/900116-1.c > /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c: > In function 'zloop': > /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14:1: > error: insn does not satisfy its constraints: > (insn 228 225 230 9 (parallel [ > (set (reg:DI 1 x1 [279]) > (unspec:DI [ > (mem/v/f/c:DI (plus:DI (reg/f:DI 31 sp) > (const_int 24 [0x18])) [4 D.2626+0 S8 A64]) > (mem/v/f/c:DI (reg/f:DI 2 x2 [277]) [4 > __stack_chk_guard+0 S8 A64]) > ] UNSPEC_SP_TEST)) > (clobber (reg:DI 2 x2 [320])) > ]) > /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14 > 741 {stack_protect_test_di} > (expr_list:REG_DEAD (reg/f:DI 13 x13 [277]) > (expr_list:REG_UNUSED (reg:DI 2 x2 [320]) > (nil)))) > /home/doko/gcc/4.9/gcc-4.9-4.9.1/src/gcc/testsuite/gcc.c-torture/compile/900116-1.c:14:1: > internal compiler error: in copyprop_hardreg_forward_1, at regcprop.c:775 > Please submit a full bug report, > with preprocessed source if appropriate. > > for now only tested with the 4.9 linaro branch, now testing with trunk. seen with trunk r215323 as well, after disabling itm (--disable-libitm) which currently doesn't seem to build. Matthias
On 17 September 2014 15:43, James Greenhalgh <james.greenhalgh@arm.com> wrote: > > On Wed, Sep 17, 2014 at 09:30:31AM +0100, Richard Earnshaw wrote: >> "=&r" is correct for an early-clobbered scratch. >> >> R. > > In that case... > > How is the attached patch for trunk? I've bootstrapped it on AArch64 > with -fstack-protector-strong and -frename-registers in the BOOT_CFLAGS > without seeing any issues. > > OK? > > Thanks, > James > > --- > gcc/ > > 2014-09-15 James Greenhalgh <james.greenhalgh@arm.com> > > * config/aarch64/aarch64.md (stack_protect_test_<mode>): Mark > scratch register as an output to placate register renaming. OK for this part. > gcc/testsuite/ > > 2014-09-15 James Greenhalgh <james.greenhalgh@arm.com> > > * gcc.target/aarch64/stack_protector_set_1.c: New. > * gcc.target/aarch64/stack_protector_set_2.c: Likewise. I agree with Andrew, these don't need to be aarch64 specific. /Marcus
On Thu, Sep 18, 2014 at 09:18:53AM +0100, Marcus Shawcroft wrote: > > gcc/testsuite/ > > > > 2014-09-15 James Greenhalgh <james.greenhalgh@arm.com> > > > > * gcc.target/aarch64/stack_protector_set_1.c: New. > > * gcc.target/aarch64/stack_protector_set_2.c: Likewise. > > I agree with Andrew, these don't need to be aarch64 specific. Well, guess the 16 needs to be replaced with sizeof buffer, because sizeof (unsigned int) is not 4 on all architectures. And /* { dg-require-effective-target fstack_protector } */ is needed, not all targets support -fstack-protector*. Jakub
On 18 September 2014 11:15, James Greenhalgh <james.greenhalgh@arm.com> wrote: > gcc/ > > 2014-09-18 James Greenhalgh <james.greenhalgh@arm.com> > > * config/aarch64/aarch64.md (stack_protect_test_<mode>): Mark > scratch register as an output to placate register renaming. > > gcc/testsuite/ > > 2014-09-18 James Greenhalgh <james.greenhalgh@arm.com> > > * gcc.dg/ssp-3.c: New. > * gcc.dg/ssp-4.c: Likewise. OK, thanks /Marcus
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index b5be79c..ed6e602 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -4026,7 +4026,7 @@ }) (define_insn "stack_protect_test_<mode>" - [(set (match_operand:PTR 0 "register_operand") + [(set (match_operand:PTR 0 "register_operand" "=r") (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") (match_operand:PTR 2 "memory_operand" "m")] UNSPEC_SP_TEST))
Hi maintainers, I just added "=r" and retested it. gcc/ChangeLog 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add register constraint for operand 0. regards, venkat. On 4 September 2014 12:42, Venkataramanan Kumar <venkataramanan.kumar@linaro.org> wrote: > Hi Maintainers, > > Below patch adds register constraint "r" for destination operand in > "stack_protect_test" pattern. > > We need a general register here and adding "r" will avoid vector > register getting allocated. > > regression tested on aarch64-unknown-linux-gnu. > > Ok for trunk? > > regards, > Venkat. > > > gcc/ChangeLog > > 2014-09-04 Venkataramanan Kumar <venkataramanan.kumar@linaro.org> > > * config/aarch64/aarch64.md (stack_protect_test_<mode>) Add register > constraint for operand 0. > > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index b5be79c..77588b9 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -4026,7 +4026,7 @@ > }) > > (define_insn "stack_protect_test_<mode>" > - [(set (match_operand:PTR 0 "register_operand") > + [(set (match_operand:PTR 0 "register_operand" "r") > (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m") > (match_operand:PTR 2 "memory_operand" "m")] > UNSPEC_SP_TEST))