[PATCH] D51861: [LSR] Combine unfolded offset into invariant register

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

Hi Gil,

Looks good to me.

Nitpicks inlined, no need for another round of review.

Cheers,
-Quentin



================
Comment at: test/Transforms/LoopStrengthReduce/two-combinations-bug.ll:3
+
+; This test is adopted from the n-body test of the LLVM test-suite: A bug in
+; r345114 caused LSR to generate incorrect code. The test verifies that the
----------------
adopted => adapted?


================
Comment at: test/Transforms/LoopStrengthReduce/two-combinations-bug.ll:43
+  %z9 = getelementptr inbounds %struct.planet.0.3.6.11.12.15.16.17.24.25.26.33.44, %struct.planet.0.3.6.11.12.15.16.17.24.25.26.33.44* %bodies, i64 %indvars.iv98, i32 2
+  %0 = load double, double* %z9, align 8, !tbaa !1
+  %indvars.iv.next99 = add nuw nsw i64 %indvars.iv98, 1
----------------
Could you get rid of the implicit variable name? (%[0-9]+)
Those are painful when manually editing tests.

`opt -instnamer` does that for you, but given you have just one, you may want to do it by hand.


================
Comment at: test/Transforms/LoopStrengthReduce/two-combinations-bug.ll:43
+  %z9 = getelementptr inbounds %struct.planet.0.3.6.11.12.15.16.17.24.25.26.33.44, %struct.planet.0.3.6.11.12.15.16.17.24.25.26.33.44* %bodies, i64 %indvars.iv98, i32 2
+  %0 = load double, double* %z9, align 8, !tbaa !1
+  %indvars.iv.next99 = add nuw nsw i64 %indvars.iv98, 1
----------------
qcolombet wrote:
> Could you get rid of the implicit variable name? (%[0-9]+)
> Those are painful when manually editing tests.
> 
> `opt -instnamer` does that for you, but given you have just one, you may want to do it by hand.
Can we get rid of the metadata?
There is an `opt -stripSomethingSomething` if you don't want to do it manually.


https://reviews.llvm.org/D51861