On Fri, Jul 23, 2021 at 7:53 AM Segher Boessenkool
<seg...@kernel.crashing.org> wrote:
>
> On Wed, Jul 14, 2021 at 05:14:16PM +0800, bin.cheng via Gcc-patches wrote:
> > Hi,
> > I ran into a wrong code bug in code with deep template instantiation when
> > working on sdx::simd.
> > The root cause as described in commit summary is we skip prologue insns in
> > init_alias_analysis.
> > This simple patch fixes the issue, however, it's hard to reduce a case
> > because of heavy use of
> > templates.
>
> > Subject: [PATCH 1/2] Don't skip prologue instructions as it could affect
> > alias
> > analysis
> >
> > In init_alias_analysis, we skip prologue instructions but this is
> > wrong as it could affect base value analysis for registers as well
> > as following analysis/transformations like cselib and dse:
> > prologue:
> > x12 = 0x1810
> > sp = sp - x12
> > ...
> > ...
> > x12 = SYMBOL_REF(.LC89)
> > Here reg_base_value[12] is set to ".LC89", however this is only true
> > after the second instruction setting x12. The patch fixes the issue
> > by just handling prologue instructions as normal. Though ideally it
> > can be improved in context-sensitive way.
>
> In what pass do you get the bad behaviour? dse2? postreload? Or what
> else?
Hi Segher,
It is dse2 deleting non dead stores based on wrong cse info again
based on wrong alias info.
The code flow is quite tricky, given insn list like:
x12 = 0x1810
sp = sp - x12
...
[sp + offset] = x
y = [sp + offset]
...
[sp + offset] = z
...
x12 = SYMBOL_REF(.LC89)
1、alias computes wrong reg_base_value[12] = symbol_ref(.LC89)
2、when dse2 process "y = [sp + offset]", the calling sequence is :
scan_insn
-> check_mem_read_rtx
-> check_mem_read_rtx
-> canon_true_dependence(group_id == -1)
-> true_dependence_1 which has below code:
------------------------------------------------------------------
base = find_base_term (x_addr);
if (base && (GET_CODE (base) == LABEL_REF
|| (GET_CODE (base) == SYMBOL_REF
&& CONSTANT_POOL_ADDRESS_P (base))))
return 0;
x_addr is "sp - x12", sp has no base term, however x12 has
symbol_ref(.LC89) base term, and the code returns 0. As a result,
dse2 considers storing x as dead when processing "[sp + offset] = z".
Sorry I can't reproduce a case out of it.
Thanks,
bin
>
> Your patch looks correct, but I'd like to know why it has seemed to work
> for so long :-)
>
>
> Segher
