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

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet requested changes to this revision.
qcolombet added a comment.
This revision now requires changes to proceed.

Hi,

> Assuming ImmCost(1022) < ImmCost(1024), but it should not as real ImmCost of the solution should be ImmCost(1022) + ImmCost(1023) which is > ImmCost(1024).

Hold on, the constants that you are using for your cost is not what the LLVM IR has when LSR does the transformation.
Assuming I compiled the test case correctly, we traverse:
getelementptr inbounds i8, i8* %arg1, i64 -1024
getelementptr inbounds i8, i8* %arg2, i64 -2
For 0 to 1024 with a +2 increment (loop partially unrolled).

The IV we get is {-1024,+,2}, that means,
First access of %arg1 is through IV + arg1
Second access of %arg1 is through PrevArg1Access + 1 // <-- this is supposed to be free by the chain profitability model
First access of %arg2 is through IV + arg2 + 1022
Second access of %arg2 is through PrevArg2Access + 1 // <-- ditto
Then the comparison is against 0

In the end, the only ImmCost that LSR sees is 1022 and 0 for that solution. I understand you'd like to add 1023 (second access of arg2) and 1 (second access of arg1), but I don't think again the averaging the fixups with the number of same base address is the right way to do it.
Instead, if you believe the cost is not free for the second accesses, I would suggest to rework the profitability check for the chain, i.e., isProfitableChain.
More particularly, this snippet:

  // Incrementing by zero or some constant is neutral. We assume constants can
  // be folded into an addressing mode or an add's immediate operand.
  if (isa<SCEVConstant>(Inc.IncExpr)) {
    ++NumConstIncrements;
    continue;
  }

Thanks,
-Quentin


Repository:
  rL LLVM

https://reviews.llvm.org/D27321