[PATCH] D132443: [LSR] Fold terminal condition to other IV when possible

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/test/Transforms/LoopStrengthReduce/lsr-fold-iv-complicate-add-rec.ll:46
+for.body.preheader:                               ; preds = %entry
+  %add.ptr = getelementptr inbounds ptr, ptr %mark, i64 3
+  br label %for.body
----------------
eopXD wrote:
> fhahn wrote:
> > is this needed?
> Thanks for dropping by. Yes, my code checks if the phi node has a value that comes from the loop pre-header. Without this the condition check won't pass.
right, but it doesn't need to be defined in the preheader, this could just be `%mark` I assume?


================
Comment at: llvm/test/Transforms/LoopStrengthReduce/lsr-fold-iv-complicate-add-rec.ll:50
+for.cond.cleanup.loopexit:                        ; preds = %for.body
+  br label %for.cond.cleanup
+
----------------
eopXD wrote:
> fhahn wrote:
> > is this needed? Would be good to try to simplify the test as much as possible. Also, it would be good to add a patch just for the tests and then only include the diff here. 
> Thanks for the reminder. Let me add a pre-commit test for this. 
It would be good to clean them up before committing the test. Also, it looks like it is missing negative tests.


================
Comment at: llvm/test/Transforms/LoopStrengthReduce/lsr-fold-iv-complicate-add-rec.ll:59
+  %0 = load ptr, ptr %dst.04, align 8
+  %call = tail call ptr @sv_2mortal(ptr %0)
+  %incdec.ptr = getelementptr inbounds ptr, ptr %dst.04, i64 1
----------------
It looks like this function doesn't need to return a pointer?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132443/new/

https://reviews.llvm.org/D132443