[PATCH] D49151: [SimplifyIndVar] Avoid generating truncate instructions with non-hoisted Load operand.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

How does the code generation actually change on aarch64?  As far as I can tell, you're basically just changing an "sxtw #2" to an "lsl #2"; does that save a uop on A72?

> Note that an alternative to this patch is to move SimplifyIndVar pass after PRE because PRE hoists more loop invariant code than LICM given that it uses a less conservative but expensive version of alias analysis.

That might work in some cases, but not in general, so this is probably worth solving anyway...



================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:1408
+bool WidenIV::widenVariantLoadUse(NarrowIVDefUse DU) {
+  const SCEV* ExtendOperExpr = getExtendExpr(DU);
+  if (!ExtendOperExpr)
----------------
Not sure the getExtendExpr helper is actually buying anything, given you have to check the extension kind anyway.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:1460
+  auto *WideBO = BinaryOperator::Create(NarrowBO->getOpcode(), LHS, RHS,
+                                        NarrowBO->getName());
+  IRBuilder<> Builder(NarrowUse);
----------------
It probably isn't a good idea to create a new multiply without erasing the old one.




================
Comment at: llvm/test/Transforms/IndVarSimplify/iv-widen-elim-ext.ll:310
+  %add.ptr1 = getelementptr inbounds i32, i32* %in, i64 %idx.ext1
+  %3 = load i32, i32* %add.ptr1, align 4
+  %cmp = icmp slt i32 %add, %length
----------------
Please fix the testcase so this load isn't dead (to make the testcase less fragile).


Repository:
  rL LLVM

https://reviews.llvm.org/D49151