[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