On Wed, Mar 7, 2012 at 10:35 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Mar 07, 2012 at 10:27:22AM +0100, Richard Guenther wrote:
>> On Tue, Mar 6, 2012 at 11:08 PM, Eric Botcazou <ebotca...@adacore.com> wrote:
>> > If you try to bootstrap the GNAT 4.7.0 compiler on IA-64/Linux with
>> > non-default
>> > options (-gnatpg replaced with -gnatpgn), you get another comparison
>> > failure
>> > caused by debug insns, stemming from the machine-specific reorg pass (aka
>> > insn
>> > bundling on IA-64). With -g , when cselib is called on:
>> >
>> > (insn 17 41 18 2 (set (reg/f:DI 14 r14 [357])
>> > (plus:DI (reg:DI 16 r16 [356])
>> > (const_int 28
>> > [0x1c]))) /home/eric/gnat.b/gnat7_47/src/gcc/ada/atree.adb:2244 205
>> > {adddi3}
>> > (expr_list:REG_DEAD (reg:DI 16 r16 [356])
>> > (nil)))
>> >
>> > it finds a previous equivalent value:
>> >
>> > (plus:DI (plus:DI (ashift:DI (value:DI 5:111 @0x29abbe0/0x29f5f20)
>> > (const_int 5 [0x5]))
>> > (value:DI 9:9 @0x29abc40/0x29f5fe0))
>> > (const_int 28 [0x1c]))
>> >
>> > computed for a debug insn:
>> >
>> > (debug_insn 12 10 65 2 (var_location:SI n (mem/j:SI (plus:DI (plus:DI
>> > (ashift:DI (reg:DI 14 r14 [orig:344 D.2979 ] [344])
>> > (const_int 5 [0x5]))
>> > (reg/f:DI 15 r15 [orig:342
>> > atree__atree_private_part__nodes__table.32 ] [342]))
>> > (const_int 28 [0x1c])) [0
>> > *atree__atree_private_part__nodes__table.32_17
>> > [D.2979_19].is_extension___XVN.S0.field5+0 S4 A8])) sem_ch2.adb:49 -1
>> > (nil))
>> >
>> > When output_dependence is called on a couple of MEMs, it uses the above
>> > value
>> > to get the equivalent addresses:
>> >
>> > (plus:DI (value:DI 9:9 @0x29abc40/0x29f5fe0)
>> > (value:DI 12:4189 @0x29abc88/0x29c8a20))
>> >
>> > and
>> >
>> > (plus:DI (plus:DI (ashift:DI (value:DI 5:111 @0x29abbe0/0x29f5f20)
>> > (const_int 5 [0x5]))
>> > (value:DI 9:9 @0x29abc40/0x29f5fe0))
>> > (const_int 28 [0x1c]))
>> >
>> > and rtx_refs_may_alias_p returns true on them because ao_ref_from_mem
>> > returns
>> > false for one of the MEMs.
>> >
>> >
>> > Without -g, when cselib is called on:
>> >
>> > (insn 14 30 15 2 (set (reg/f:DI 14 r14 [357])
>> > (plus:DI (reg:DI 16 r16 [356])
>> > (const_int 28
>> > [0x1c]))) /home/eric/gnat.b/gnat7_47/src/gcc/ada/atree.adb:2244 205
>> > {adddi3}
>> > (expr_list:REG_DEAD (reg:DI 16 r16 [356])
>> > (nil)))
>> >
>> > output_dependence only gets the equivalent addresses:
>> >
>> > (plus:DI (value:DI 8:8 @0x299f2e8/0x299f1a0)
>> > (value:DI 10:4188 @0x299f318/0x299f200))
>> >
>> > and
>> >
>> > (plus:DI (value:DI 12:4254 @0x299f348/0x29a0490)
>> > (const_int 28 [0x1c]))
>> >
>> > and memrefs_conflict_p is able to prove that they don't conflict.
>> >
>> >
>> > The problem is that the more complex expression in the first case fools
>> > memrefs_conflict_p because the predicate makes a wrong assumption about the
>> > canonicalization of address expressions. Hence the attached patch.
>> >
>> > Bootstrapped/regtested on IA-64/Linux, OK for the mainline? Do we also
>> > want it
>> > for 4.7.1 or is it too specific?
>>
>> Hmm, but isn't the bug that we feed debug-insn mems to memrefs_conflict_p?
>> Or that we have non-legitimate address expressions in them?
>
> CCing Alex. I think we feed debug insn mems in the scheduler to be able to
> find out what debug insns need to be invalidated and what can be kept.
> And any address expressions are legitimate for debug insns, why should we be
> limited by what the ISA allows? All we are limited is if we can express
> those expressions in DWARF or not.
> How can this be reproduced with a cross?
Hmm, but then this complicates and slows down the generic alias machinery.
Of course IMHO the RTL alias machinery should be conservative with respect
to what the RTL IL allows - so the question is are non-legitimate addresses
valid in regular instructions at any point?
Richard.
>> > 2012-03-06 Eric Botcazou <ebotca...@adacore.com>
>> >
>> > * alias.c (memrefs_conflict_p) <PLUS>: Correct wrong assumption
>> > about
>> > canonicalization of address expressions.
>
> Jakub
