On Wed, Mar 23, 2016 at 01:03:18AM +0100, Bernd Schmidt wrote: > On 03/21/2016 02:43 AM, Alan Modra wrote: > > > >+enum valid_equiv { valid_none, valid_combine, valid_reload }; > >+ > > Might be worth documenting that each step represents a superset of the > previous one.
Fixed. > >+ ret = valid_combine; > >+ if (! MEM_READONLY_P (memref) > >+ && ! RTL_CONST_OR_PURE_CALL_P (insn)) > >+ return valid_none; > >+ } > > The gcc style is actually not to have a space after unary "!". None of the > code in this file follows that, but I think you may want to change that as > you modify things in your patches, and have new code follow the recommended > style. Fixed. > >@@ -3536,7 +3557,8 @@ update_equiv_regs (void) > > { > > /* Note that the statement below does not affect the priority > > in local-alloc! */ > >- REG_LIVE_LENGTH (regno) *= 2; > >+ if (note) > >+ REG_LIVE_LENGTH (regno) *= 2; > > That's a very suspicious comment. It would be worth testing whether > REG_LIVE_LENGTH has any effect on our current register allocation at all, > and remove this code if not. Yes, REG_LIVE_LENGTH is used in just one place in the whole of gcc, and that's the test in update_equiv_regs just above the code you quote. /* Don't mess with things live during setjmp. */ if (REG_LIVE_LENGTH (regno) >= 0 && optimize) That could be replaced with if (optimize && !bitmap_bit_p (setjmp_crosses, regno)) and outside the loop bitmap setjmp_crosses = regstat_get_setjmp_crosses (); For now, I've removed the REG_LIVE_LENGTH adjustment from patch 7/7. I'll also prepare a patch to delete REG_LIVE_LENGTH everywhere. -- Alan Modra Australia Development Lab, IBM