On 01/03/16 10:49, Richard Biener 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  <rguent...@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?
> 

Because b is the mnemonic for a branch instruction.  What follows is
probably garbage in reality if you start from that point.

> IMHO the warning is simply bogus?

I think the grammar for the '=' assignment is at best dubious, given
that the LHS could be any label and there's no guarantee that won't
conflict with a mnemonic.  .set is clearly superior in that respect.

R.

> 
> 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.
>>>
>>
>>
> 

Reply via email to