Hi! On Fri, Jun 11, 2021 at 09:16:21PM +0800, Kewen.Lin wrote: > >> + /* Should pick up the lowest luid if the references > >> + are in the same block. */ > >> + if (label_tick == rsp->last_set_table_tick > >> + && rsp->last_set_table_luid > insn_luid) > >> + rsp->last_set_table_luid = insn_luid; > > > > Why? Is it conservative for the check you will do later? Please spell > > this out, it is crucial! > > Since later the combinations involving this insn probably make the > register be used in one insn sitting ahead (which has smaller luid than > the one which was recorded before). Yes, it's very conservative, this > ensure that we always use the luid of the insn which is the first insn > using this register in the block.
Why would that be correct?! > The last_set invalidation is going > to catch the case like: > > ... regX // avoid the set used here ... > regX = ... > ... > > Once we have the smallest luid one of all insns which use register X, > any unsafe regX sets should be caught. Yes, you invalidate more, but because you put lies in the table :-( > * combine.c (struct reg_stat_type): New member > last_set_table_luid. This fits on one line. > (update_table_tick): Add one argument for insn luid and > set last_set_table_luid with it, remove its declaration. > (record_value_for_reg): Adjust the condition to set > last_set_invalid nonzero. These lines break earlier than they should as well. > + /* Record the luid of the insn which uses register n, the insn should > + be the first one using register n in that block of the insn which > + last_set_table_tick was set for. */ > + > + int last_set_table_luid; I'm not sure what this variable is for. The comment says something else than the variable name does, and now I don't know what to believe :-) The name says it is for a SET, the explanation says it is for a USE. Segher
