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 <pins...@gmail.com> wrote: > On Thu, Sep 4, 2014 at 1:18 AM, James Greenhalgh > <james.greenha...@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.ku...@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.ku...@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.ku...@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)) >>>
