[PATCH/RFC] combine: Tweak the condition of last_set invalidation

[Segher Boessenkool]

Hi Ke Wen,

On Fri, Jan 15, 2021 at 04:06:17PM +0800, Kewen.Lin wrote:
> on 2021/1/15 上午8:22, Segher Boessenkool wrote:
> > On Wed, Dec 16, 2020 at 04:49:49PM +0800, Kewen.Lin wrote:
> >>    ... op regX  // this regX could find wrong last_set below
> >>    regX = ...   // if we think this set is valid
> >>    ... op regX
> >>
> >> But because of retry's existence, the last_set_table_tick could
> > 
> > It is not just because of retry: combine can change other insns than
> > just i2 and i3, too.  And even changing i2 requires this!
> 
> Ah, thanks for the information!  Here retry is one example for that
> we can revisit one instruction but meanwhile the stored information
> for reg reference can be from that instruction after the current
> one but visited before.

Yes; and we do not usually revisit just one insn, but everything after
it as well.  We only need to revisit thos insns that are fed by what
has changed, but this is a good enough approximation (we never revisit
very far back).

> > The whole reg_stat stuff is an ugly hack that does not work well.  For
> > example, as in your example, some "known" value can be invalidated
> > before the combination that wants to know that value is tried.
> > 
> > We need to have this outside of combine, in a dataflow(-like) thing
> > for example.  This could take the place of REG_EQ* as well probably
> > (which is good, there are various problems with that as well).
> 
> Good point, but IIUC we still need to keep updating(tracking)
> information like what we put into reg_stat stuff, it's not static
> since as you pointed out above, combine can change i2/i3 etc,
> we need to update the information for the changes.

Yes, we should keep it correct all the time, and for any point in the
code.  It also can be used by other passes, e.g. it can replace all
REG_EQ* notes, all nonzero_bits and num_sign_bit_copies, and no doubt
even more things.

> Anyway, it's not what this patch tries to solve.  :-P

:-)

> >> This proposal is to check whether the last_set_table safely happens
> >> after the current set, make the set still valid if so.
> > 
> > I don't think this is safe to do like this, unfortunately.  There are
> > more places that set last_set_invalid (well, one more), so at the very
> > minimum this needs a lot more justification.
> 
> Let me try to explain it more.
> * Background *
> 
> There are two places which set last_set_invalid to 1. 
> 
> CASE 1:

<SNIP>

Thanks for the in-depth explanation!

I think this should be postponed to stage 1 though?  Or is there
anything very urgent in it?


Segher