Hi Segher, on 2021/1/22 上午8:30, Segher Boessenkool wrote: > 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? >
Yeah, I agree that this belongs to stage1, and there isn't anything urgent about it. Thanks for all further comments above! BR, Kewen
