On 9/11/19 12:43 AM, Jeff Law wrote:
> On 9/10/19 1:51 PM, Bernd Edlinger wrote:
>> Hi!
>>
>> This ICE happens when compiling real_nextafter in real.c.
>> CSE sees this:
>>
>> (insn 179 178 180 11 (set (reg:SI 319)
>> (reg/v/f:SI 273 [ rD.73757 ])) "../../gcc-trunk-1/gcc/real.c":120:10
>> 643 {*thumb2_movsi_vfp}
>> (nil))
>> [...]
>> (insn 181 180 182 11 (set (mem:SI (reg:SI 319) [0 MEM <charD.7[1:24]>
>> [(voidD.73 *)r_77(D)]+0 S4 A8])
>> (unspec:SI [
>> (reg:SI 320)
>> ] UNSPEC_UNALIGNED_STORE)) "../../gcc-trunk-1/gcc/real.c":120:10
>> 129 {unaligned_storesi}
>> (nil))
>> [...]
>> (insn 186 185 187 11 (set (mem:SI (plus:SI (reg/v/f:SI 273 [ rD.73757 ])
>> (const_int 20 [0x14])) [0 MEM <charD.7[1:24]> [(voidD.73
>> *)r_77(D)]+20 S4 A8])
>> (unspec:SI [
>> (reg:SI 320)
>> ] UNSPEC_UNALIGNED_STORE)) "../../gcc-trunk-1/gcc/real.c":120:10
>> 129 {unaligned_storesi}
>> (expr_list:REG_DEAD (reg:SI 320)
>> (expr_list:REG_DEAD (reg/f:SI 319 [ rD.73757 ])
>> (nil))))
>> [...]
>> (insn 234 233 235 11 (set (reg:SI 340)
>> (mem:SI (reg/v/f:SI 273 [ rD.73757 ]) [52 MEM <unsigned int>
>> [(struct real_valueD.28367 *)r_77(D)]+0 S4 A32]))
>> "../../gcc-trunk-1/gcc/real.c":5185:9 643 {*thumb2_movsi_vfp}
>> (nil))
>>
>>
>> ... and transforms insn 234 in an invalid insn:
>>
>>
>> (insn 234 233 235 11 (set (reg:SI 340 [ MEM <unsigned int> [(struct
>> real_valueD.28367 *)r_77(D)] ])
>> (mem:SI (plus:SI (reg/v/f:SI 273 [ rD.73757 ])
>> (const_int 20 [0x14])) [0 MEM <charD.7[1:24]> [(voidD.73
>> *)r_77(D)]+20 S4 A8])) "../../gcc-trunk-1/gcc/real.c":5185:9 643
>> {*thumb2_movsi_vfp}
>> (nil))
>>
>> which triggers the assertion in the arm back-end, because the MEM is not
>> aligned.
>>
>> To fix that I changed exp_equiv_p to consider MEMs with different MEM_ALIGN
>> or
>> ALIAS_SET as different.
>>
>> This patch fixes the arm bootstrap for --with-cpu=cortex-a57
>> --with-mode=thumb --with-fpu=fp-armv8 --with-float=hard
>> which I confirmed using a cross compiler. And it fixes the test case that
>> is attached to the PR, but it is way
>> too large for the test suite.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>>
>> patch-pr91708.diff
>>
>> 2019-09-10 Bernd Edlinger <bernd.edlin...@hotmail.de>
>>
>> PR middle-end/91708
>> * cse.c (exp_equiv_p): Consider MEMs with different
>> alias set or alignment as different.
> Hmm, I would have expected the address space and alignment checks to be
> handled by the call to mem_attrs_eq_p.
>
Yes, but exp_equiv_p is called from here:
lookup (rtx x, unsigned int hash, machine_mode mode)
{
struct table_elt *p;
for (p = table[hash]; p; p = p->next_same_hash)
if (mode == p->mode && ((x == p->exp && REG_P (x))
|| exp_equiv_p (x, p->exp, !REG_P (x), false)))
return p;
with for_gcse = false si mem_attrs_eq_p is not used.
> Which aspect of the MEMs is really causing the problem here?
>
The alignment is (formally) different, but since also the address is different,
the alignment could be also be really wrong.
Originally it was [(struct real_valueD.28367 *)r_77(D)]+0 S4 A32]
but it is replaced with [0 MEM <charD.7[1:24]> [(voidD.73 *)r_77(D)]+20 S4 A8],
that appears to be done, because there is the same value that was in the
original
location.
But also the wrong alias set will cause problems if the instruction
is re-materialized in another place (which actually happened in LRA and caused
the
assertion, BTW).
Bernd.
