This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v2 AArch64]: Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern.
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: Andrew Pinski <pinskia at gmail dot com>
- Cc: Venkataramanan Kumar <venkataramanan dot kumar at linaro dot org>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Patch Tracking <patch at linaro dot org>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, doko at ubuntu dot com, steve dot mcintyre at linaro dot org
- Date: Tue, 16 Sep 2014 23:03:44 +0100
- Subject: Re: [PATCH v2 AArch64]: Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern.
- Authentication-results: sourceware.org; auth=none
- References: <CAJK_mQ1NB7gfroQuAi+H_+TXzb51KPaMK2EsEoFFswvTL4fCFQ at mail dot gmail dot com> <20140904081855 dot GA18028 at arm dot com> <CA+=Sn1nnBPtYycQTri3-7_P6KjyQjgYQ_+KCUODoqaK=NaVJoA at mail dot gmail dot com>
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