[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