[PATCH] D53612: [LV] Avoid vectorizing loops under opt for size that involve SCEV checks
Dorit Nuzman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 24 13:56:27 PDT 2018
dorit accepted this revision.
dorit added a comment.
This revision is now accepted and ready to land.
Just minor comments on the tests.
LGTM.
================
Comment at: test/Transforms/LoopVectorize/pr30654-phiscev-sext-trunc.ll:245
+; Check that the need for overflow check prevents vectorizing a loop with tiny
+; trip count / under opt for size.
+; CHECK-LABEL: @func_34
----------------
"with tiny trip count / under opt for size" --> "with tiny trip count (which implies opt for size).".
================
Comment at: test/Transforms/LoopVectorize/pr30654-phiscev-sext-trunc.ll:250
+; CHECK-LABEL: bb67:
+define void @func_34() {
+bb1:
----------------
(can mention now that this is pr39417)
================
Comment at: test/Transforms/LoopVectorize/pr30654-phiscev-sext-trunc.ll:254
+
+bb67: ; preds = %bb67, %bb1
+ %storemerge2 = phi i32 [ 0, %bb1 ], [ %_tmp2300, %bb67 ]
----------------
I was once asked to remove the "; preds = ..." comments if they are not needed. (I see a lot of tests including these comments, but this specific file doesn't).
================
Comment at: test/Transforms/LoopVectorize/pr30654-phiscev-sext-trunc.ll:267
+
+; Check that the need for stride==1 check prevents vectorizing a loop under opt
+; for size.
----------------
Now that there's a PR opened for this scenario, you may want to consider moving these two loops to a separate file (pr39417-optsize-and-scevchecks.ll ?); I know that there's a desire to not open separate files for each testcase, but this specific file deals with sext-trunc overflow tests (as the name of the test implies), so may be less appropriate to include the stride==1 testcase here?
Repository:
rL LLVM
https://reviews.llvm.org/D53612
More information about the llvm-commits
mailing list