[PATCH] D27321: Fix LSR ImmCost calculation for profitable chains

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

Hi,

> As there is no exact Offset (see above) we can only estimate.

That part bothered me in the first place. I don't see how we can derive good heuristics by injecting guesses in the input data. It sounds to me that we rely even more on the luck factor.

Looking a bit closer, I agree that the main problem is that we assume the fix ups are free for the profitable chain. Maybe we should change the profitability check for the chains altogether or have a different user for them, like one that only sums the immcost?
In particular, the code below that line could use some refinement:

  // Incrementing by zero or some constant is neutral. We assume constants can
  // be folded into an addressing mode or an add's immediate operand.

That being said, I believe we have another bug in the profitable computation. Namely, I believe this part

  // An IV chain with a single increment is handled by LSR's postinc
  // uses. However, a chain with multiple increments requires keeping the IV's
  // value live longer than it needs to be if chained.
  if (NumConstIncrements > 1)
    --cost;

Should be turned into:

  // An IV chain with a single increment is handled by LSR's postinc
  // uses. However, a chain with multiple increments requires keeping the IV's
  // value live longer than it needs to be if chained.
  if (NumConstIncrements > 1)
    ++cost;

I.e., having to keep something around should increase the cost of the chain, not decreasing it.

Fixing that seems to fix your test case, but I may have overlooked something admittedly.

Cheers,
-Quentin


Repository:
  rL LLVM

https://reviews.llvm.org/D27321