[PATCH] D128523: [LV][NFC] Fix the condition for printing debug messages

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 8 09:31:48 PDT 2022


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

Thanks for the update! Just a few comments with regard to the test.

LGTM, with the suggested cleanup for the test. It would also be good to pre-commit the test and then have the diff only show the change caused by the patch.



================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/force-vect-msg.ll:14
+;
+; The test case is from:
+;
----------------
Given that the LLVM IR is very compact I don't think the C code adds much and IMO it would be better to not include it.


================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/force-vect-msg.ll:27
+;
+; Regardless of force vectorization or not, this loop will eventually be vectorized because of the cost model.
+; Therefore, the following message does not need to be printed even if vectorization is explicitly forced in the metadata.
----------------
Keep the comment next to the check?


================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/force-vect-msg.ll:34
+entry:
+  %cmp4.not = icmp eq i64 %N, 0
+  br i1 %cmp4.not, label %for.cond.cleanup, label %for.body.preheader
----------------
this should not be needed, you can branch directly to either %for.body.preheader or %for.body.


================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/force-vect-msg.ll:40
+
+for.cond.cleanup:                                 ; preds = %for.body, %entry
+  %rd.0.lcssa = phi i64 [ %init, %entry ], [ %add, %for.body ]
----------------
IMO the test is easier to read if this exit block is at the end of the function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128523



More information about the llvm-commits mailing list