[PATCH] D21398: [LSR] Use post-inc expression for post-inc user

[Andrew Trick via llvm-commits]

atrick added a comment.

This is definitely a case worth fixing.

The "problem" with this case from LSR point of view is that SCEV is
too smart and hoists the 'trunc' out of the recurrence. This makes
it look like the expression that feeds the store and the compare are
based on different initial values.

This patch looks like it goes through a fairly involved analysis
before LSR to undo the normalization that happens during IVUsers. I
think that only obscures the input to LSR. It happens to generate
better code in this case, but LSR still thinks that two registers are
needed for the loop counter. So LSR hasn't really been fixed, and I'm
afraid this will prevent LSR from optimizing other cases.

Look at the output of LSR *after* applying this patch:

The chosen solution requires 3 regs, with addrec cost 2, plus 1 scale cost, plus 1 setup cost:

  LSR Use: Kind=Basic, Offsets={0}, widest fixup type: i64
    reg({(sext i32 %n to i64),+,1}<%for.body>)
  LSR Use: Kind=Basic, Offsets={0}, widest fixup type: i32
    reg({%n,+,1}<%for.body>)
  LSR Use: Kind=Address of i32 in addrspace(0), Offsets={0}, widest fixup type: i32*
    reg(@gvarray) + 4*reg({(sext i32 %n to i64),+,1}<%for.body>)

So the problem has not been fixed in LSR. We just get lucky and "replaceCongruentPhis" happens to cleanup the extra IV:

INDVARS: Eliminated congruent iv.inc:   %lsr.iv.next = add i32 %lsr.iv, 1
INDVARS: Eliminated congruent iv:   %lsr.iv = phi i32 [ %lsr, %for.body ], [ %n, %for.body.preheader ]

I have two suggestions. I'm not sure either will work:

(1) Fix IVUsers so that the truncate is itself the IV users. Maybe do this only if the trunc has a single user and/or the use it itself not an "interesting" SCEV. Then LSR will see the same SCEV recurrence for all users. This will be easy, but I don't know if it will mess up any other cases.

(2) Improve replaceCongruentPhis to cleanup these sort of off-by-one-with-postinc-user phis. This requires a fair amount of pattern matching.

In any case, the important, and time-consuming part will be evaluating AArch64 performance on the LLVM test suite after the change. Although if there are few changes, it may be sufficient just to look at the disassembly diffs to see that they're reasonable.


http://reviews.llvm.org/D21398