Re: 0001-Don-t-skip-prologue-instructions-as-it-could-affect-.patch

["Bin.Cheng" <amker.cheng@gmail.com>]

On Fri, Jul 23, 2021 at 7:53 AM Segher Boessenkool
<segher@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