[PATCH] D24557: [Loop Vectorizer] Simplified GEP cloning. NFC.

Matthew Simpson via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 11:24:27 PDT 2016


mssimpso added a comment.

In https://reviews.llvm.org/D24557#542757, @mkuper wrote:

> Just to make sure I understood correctly, the reason this works is that for a consecutive GEP, each operand must be either uniform or consecutive (this holds for both ptr and non-ptr IVs), so getScalarValue(0, 0) always does the right thing?


Right, for a consecutive GEP, only the induction operand (as in getGEPInductionOperand) should be loop-varying. The other operands should be uniform.

> The only thing I'm a bit concerned about is removing all the asserts. Perhaps it's worth adding a loop under #ifndef NDEBUG, that verifies this? But if you/Ayal/Matt don't think it's useful, I won't insist.

>  Other than that, LGTM.


Yeah, I agree about the asserts. We check these conditions in isConsecutivePtr. But just to mirror what what you said above, I think we should be able to verify them with following assert in the existing loop:

  assert(i == getGEPInductionOperand(Gep) || Legal->isUniform(Gep->getOperand(i))

What do you think?


Repository:
  rL LLVM

https://reviews.llvm.org/D24557





More information about the llvm-commits mailing list