[PATCH v2 AArch64]: Re: [PATCH AArch64]: Add constraint letter for stack_protect_test pattern.

Andrew Pinski pinskia@gmail.com
Tue Sep 16 22:25:00 GMT 2014


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
>



More information about the Gcc-patches mailing list