On Wed, Jun 28, 2017 at 01:49:26PM +0100, Wilco Dijkstra wrote: > Ramana Radhakrishnan wrote: > > > > I'm about to run home for the day but this came in from > > https://gcc.gnu.org/ml/gcc-patches/2013-09/msg02109.html and James > > said in that email that this was put in to ensure no segfaults on > > cortex-a15 / cortex-a7 tuning. > > The code is historical - an older version didn't specifically look just for > shifts. Given we can only return shift rtx's now it's completely redundant. > A bootstrap on armhf with --with-cpu=cortex-a15 passed.
The final commit was: https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00533.html That ends up being a more involved rewrite of the code in arm_no_early_alu_shift_dep . The check Wilco deletes was in their before the clean-up. Previosuly we were manually walking the RTX for the consumer, assuming that after stripping COND_EXEC and PARALLEL we'd have either: OP = (some_shift (register) (*)) or OP = (some_arithmetic_code (some_shift (register) (*)) (*)) That is, we're either looking at a shift, or an arithmetic operation that contains a shift. The early_op = XEXP (op, 0); would give you either: EARLY_OP = (regiser) or EARLY_OP = (some_shift (register) (*)) But, we're interested in getting to the whole shift. So, we check if we are now looking at a register, and if we are, use op instead: if (REG_P (early_op)) early_op = op; So we end up with either: EARLY_OP = (some_shift (register) (*)) or EARLY_OP = (some_shift (register) (*)) Which is exactly what we wanted. After the refactoring, arm_find_shift_sub_rtx will always return you (some_shift (*) (*)) - that's good we're now more resilient, and always operate on a full shift, but it does mean the check for REG_P can never match. I wrote this a fair while ago, but it looks like a simple oversight. So, this code is safely dead and can be cleaned up as Wilco suggests with no issue. Hope that helps! Thanks, James
