This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR69951
- From: Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Ramana Radhakrishnan <ramana dot radhakrishnan at foss dot arm dot com>, James Greenhalgh <james dot greenhalgh at arm dot com>, gcc Patches <gcc-patches at gcc dot gnu dot org>, Kyrylo Tkachov <kyrtka01 at arm dot com>, Richard Earnshaw <rearnsha at arm dot com>, Ramana Radhakrishnan <ramrad01 at arm dot com>, nickc at redhat dot com
- Date: Tue, 1 Mar 2016 21:31:03 +0530
- Subject: Re: [PATCH] Fix PR69951
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1602260930480 dot 31547 at t29 dot fhfr dot qr> <20160229173637 dot GA33430 at arm dot com> <alpine dot LSU dot 2 dot 11 dot 1603011017200 dot 31547 at t29 dot fhfr dot qr> <20160301095149 dot GA20934 at arm dot com> <alpine dot LSU dot 2 dot 11 dot 1603011053120 dot 31547 at t29 dot fhfr dot qr> <56D57365 dot 1040103 at foss dot arm dot com> <alpine dot LSU dot 2 dot 11 dot 1603011149050 dot 31547 at t29 dot fhfr dot qr>
On 1 March 2016 at 16:19, Richard Biener <rguenther@suse.de> wrote:
> On Tue, 1 Mar 2016, Ramana Radhakrishnan wrote:
>
>>
>>
>> On 01/03/16 09:54, Richard Biener wrote:
>> > On Tue, 1 Mar 2016, James Greenhalgh wrote:
>> >
>> >> On Tue, Mar 01, 2016 at 10:21:27AM +0100, Richard Biener wrote:
>> >>> On Mon, 29 Feb 2016, James Greenhalgh wrote:
>> >>>
>> >>>> On Fri, Feb 26, 2016 at 09:32:53AM +0100, Richard Biener wrote:
>> >>>>>
>> >>>>> The following fixes PR69951, hopefully the last case of decl alias
>> >>>>> issues with alias analysis. This time it's points-to and the DECL_UIDs
>> >>>>> used in points-to sets not being canonicalized.
>> >>>>>
>> >>>>> The simplest (and cheapest) fix is to make aliases refer to the
>> >>>>> ultimate alias target via their DECL_PT_UID which we conveniently
>> >>>>> have available.
>> >>>>>
>> >>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>> >>>>>
>> >>>>> Richard.
>> >>>>>
>> >>>>> 2016-02-26 Richard Biener <rguenther@suse.de>
>> >>>>>
>> >>>>> PR tree-optimization/69551
>> >>>>> * tree-ssa-structalias.c (get_constraint_for_ssa_var): When
>> >>>>> looking through aliases adjust DECL_PT_UID to refer to the
>> >>>>> ultimate alias target.
>> >>>>>
>> >>>>> * gcc.dg/torture/pr69951.c: New testcase.
>> >>>>
>> >>>> I see this new testcase failing on an ARM target as so:
>> >>>>
>> >>>> /tmp/ccChjoFc.s: Assembler messages:
>> >>>> /tmp/ccChjoFc.s:21: Warning: [-mwarn-syms]: Assignment makes a symbol match an ARM instruction: b
>> >>>>
>> >>>> FAIL: gcc.dg/torture/pr69951.c -O0 (test for excess errors)
>> >>>>
>> >>>> But I haven't managed to reproduce it outside of the test environment.
>> >>>>
>> >>>> The fix looks trivial, rename b to anything else you fancy (well... stay
>> >>>> clear of add and ldr). I'll put a fix in myself if I can manage to get
>> >>>> this to reproduce - though if anyone else wants to do it I won't be
>> >>>> offended :-).
>> >>>
>> >>> Huh, I wonder what's the use of such warning. After all 'ldr' is a valid
>> >>> C symbol name, too. In fact my cross arm as doesn't report this
>> >>> warning (binutils 2.25.0)
>> >>>
>> >>>> arm-suse-linux-gnueabi-as t.s -mwarn-syms
>> >>> Assembler messages:
>> >>> Error: unrecognized option -mwarn-syms
>> >>
>> >> Right, I've figured out the set of conditions... You need to be targeting
>> >> an arm-*-linux-* system to make sure that the ASM_OUTPUT_DEF definition
>> >> from config/arm/linux-elf.h is pulled in. This causes us to emit:
>> >>
>> >> b = a
>> >>
>> >> Rather than
>> >>
>> >> .set b,a
>> >>
>> >> Writing it as "b = a" causes the warning added to resolve binutils
>> >> PR18347 [1] to kick in, so you need binutils > 2.26 or to have backported
>> >> that patch).
>> >>
>> >> Resolving it by hacking the testcase would be one fix, but I wonder why the
>> >> ARM port prefers to emit "b = a" in a linux environment if .set does the
>> >> same thing and always avoids the warning? Maybe Ramana/Richard/Kyrill/Nick
>> >> remember?
>> >> (AArch64 does the same thing, but the AArch64 gas port doesn't
>> >> have the PR18347 fix).
>> >
>> > So does b = a define a macro then and the warning is to avoid you
>> > doing
>>
>>
>>
>>
>> I don't think this is a macro, b = a seems to be a way of setting the
>> value of a to b. in the assembler. If a is an expression , then I
>> believe the expression is resolved at assemble time - (b ends up being a
>> symbol in the symbol table produced with the value of a) in this case
>> the address of a. .set b, a achieves the same thing from my experiments
>> and reading of the sources. The reason ports appear to choose not to use
>> the .set a, b idiom is if the assembler syntax has hijacked the .set
>> directive for something else. Thus I don't see why we use the
>> ASM_OUTPUT_DEF form in the GNU/Linux port TBH rather than the .set form
>> especially as we don't reuse .set for anything else in the ARM assembler
>> port and SET_ASM_OP is defined in config/arm/aout.h.
>>
>> The use of .set in the arm port of glibc for assembler code for the same
>> purpose seems to also vindicate that kind of thought.
>> No reasons were given here[1], maybe Nick or Richard remember from
>> nearly 18 years ago ;)
>>
>>
>> Therefore this seems to be an assembler bug to me in that it doesn't
>> allow such an assignment of values, and a backend wart to me that we
>> have ASM_OUTPUT_DEF defined for no good reason. So, a patch that removes
>> ASM_OUTPUT_DEF from linux-elf.h seems obvious to me pending testing.
>>
>>
>> Nick , Richard - any thoughts ?
>
> So - why does it warn at all for this? And why does it only warn
> for b = a and not .set b, a?
The rationale for that appears to be in comment 3 for binutils PR18347:
https://sourceware.org/bugzilla/show_bug.cgi?id=18347
"As far as adding some way to suppress the warning... Instruction set
extensions mean
that an acceptable symbol one day will cause a warning tomorrow.
Having some way to suppress
the warning would be good. If it's possible, I'd suggest keeping the
warning on definitions of the form "X=Y",
and having no warning on the rather more explicit (but equivalent,
right?) ".set X, Y".
If the user really wants a symbol with that kind of name, they
oughtn't mind being explicit about it.
Thanks,
Prathamesh
>
> IMHO the warning is simply bogus?
>
> Richard.
>
>>
>> regards
>> Ramana
>>
>> 1. https://gcc.gnu.org/ml/gcc-patches/1998-10/msg00701.html
>>
>> >
>> > ...
>> >
>> > ldr 0, 1 (or whatever correct ldr instruction)
>> >
>> > and have that ldr replaced by b?
>> >
>> > Then it's a bug to emit aliases in this form and I hope .set ldr, b
>> > doesn't have the same effect.
>> >
>> > Richard.
>> >
>>
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)