[PATCH] D146974: [test] A test case for D146958

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:7
+; CHECK: vector.body
+for.cond1.preheader.lr.ph:
+  %arrayidx7 = getelementptr inbounds i64, ptr %dims1, i64 1
----------------
`entry`?


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:10
+  %arrayidx11 = getelementptr inbounds i64, ptr %dims2, i64 1
+  %0 = load i64, ptr %arrayidx7, align 8
+  %cmp845 = icmp sgt i64 %0, 0
----------------
Is this load needed or can the value directly be passed as argument?


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:14
+
+for.cond6.preheader.lr.ph:                        ; preds = %for.cond.cleanup4, %for.cond1.preheader.lr.ph
+  %1 = phi i64 [ 0, %for.cond1.preheader.lr.ph ], [ %inc26, %for.cond.cleanup4 ]
----------------
Please shorten the block names and try to make them more descriptive, this will make the test easier to read. E.g. IIUC this could be something like `loop1.header`.




================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:17
+  %2 = getelementptr double, ptr %pin, i64 %1
+  br i1 %cmp845, label %for.cond6.preheader.us.preheader, label %for.cond.cleanup4
+
----------------
Is this branch needed here?


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:20
+for.cond6.preheader.us.preheader:                 ; preds = %for.cond6.preheader.lr.ph
+  %.pre = load i64, ptr %arrayidx11, align 8
+  %mul14.us = mul nsw i64 %1, %.pre
----------------
Is this load needed or can the value directly be passed as argument?


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:25
+
+for.cond6.preheader.us:                           ; preds = %for.cond6.for.cond.cleanup9_crit_edge.us, %for.cond6.preheader.us.preheader
+  %i.048.us = phi i64 [ %inc23.us, %for.cond6.for.cond.cleanup9_crit_edge.us ], [ 0, %for.cond6.preheader.us.preheader ]
----------------
this could be `loop2.header`.


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:30
+  %mul19.us = mul nsw i64 %i.048.us, %0
+  br label %for.body10.us
+
----------------
is this branch needed?


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:32
+
+for.body10.us:                                    ; preds = %for.cond6.preheader.us, %for.body10.us
+  %j.046.us = phi i64 [ 0, %for.cond6.preheader.us ], [ %inc.us, %for.body10.us ]
----------------
This could be `loop3.header`. Is a 3 level loop nest needed for the test? Would a 2 level loop nest be sufficient?


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:45
+
+for.cond6.for.cond.cleanup9_crit_edge.us:         ; preds = %for.body10.us
+  %inc23.us = add nuw nsw i64 %i.048.us, 1
----------------
`loop2.latch`?


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:50
+
+for.cond.cleanup4:                                ; preds = %for.cond6.for.cond.cleanup9_crit_edge.us, %for.cond6.preheader.lr.ph
+  %inc26 = add nuw nsw i64 %1, 1
----------------
`loop1.latch`


================
Comment at: llvm/test/Transforms/LoopVectorize/vector-no-scevcheck.ll:55
+
+for.cond.cleanup:                                 ; preds = %for.cond.cleanup4
+  ret void
----------------
`exit`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146974