This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR69951


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)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]