[PATCH] D33055: [LoopOptimizer][Fix]PR32859, PR24738

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4349
     if (LCSSAPhi->getNumIncomingValues() == 1)
-      LCSSAPhi->addIncoming(UndefValue::get(LCSSAPhi->getType()),
+        LCSSAPhi->addIncoming(LCSSAPhi->getIncomingValue(0),
                             LoopMiddleBlock);
----------------
This makes sense to me. My only suggestion might would be to add an assert that LCSSAPhi->getIncomingValue(0) is loop-invariant in the original loop. We allow external uses of inductions and reductions, but we fix those phis differently before calling fixLCSSAPHIs. So if a phi doesn't yet have an entry for the middle block by the time we get to fixLCSSAPHIs, we know the incoming value should be loop-invariant.


================
Comment at: test/Transforms/LoopVectorize/pr32859.ll:1
+; RUN: opt < %s -loop-vectorize -S 2>&1 | FileCheck %s
+
----------------
davide wrote:
> And add a comment explaining what this test does/fixes.
You don't need the "2>&1"


================
Comment at: test/Transforms/LoopVectorize/pr32859.ll:26-30
+attributes #0 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.ident = !{!0}
+
+!0 = !{!"clang version 5.0.0 (http://llvm.org/git/clang.git 08c94e4bd75c5a2e4de5ea108d98739f5179dbeb) (http://llvm.org/git/llvm.git 7d66f52694bdfabf0081a1d40b99fca0c189a988)"}
----------------
davide wrote:
> I think you can probably get rid of some attributes.
You don't need the attributes and metadata.


https://reviews.llvm.org/D33055