[PATCH] D73490: [LV] Remove nondeterminacy by changing LoopVectorizationLegality::Reductions from DenseMap to MapVector

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added reviewers: Ayal, gilr.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM thanks. I've added a few nits to the test.



================
Comment at: llvm/test/Transforms/LoopVectorize/reduction-order.ll:5
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-grtev4-linux-gnu"
+
----------------
If the test is x86 specific, please move to LoopVectorize/X86. But it is probably enough to drop the triple and pass -force-vector-width= and -force-vector-interleave directly.


================
Comment at: llvm/test/Transforms/LoopVectorize/reduction-order.ll:11
+; CHECK: vector.body:
+; CHECK: %[[VAR1:.*]] = add <4 x i32>
+; CHECK-NEXT:  = add <4 x i32>
----------------
Given that test explicitly cares about ordering I think it would be better to include the operands for the adds as well.


================
Comment at: llvm/test/Transforms/LoopVectorize/reduction-order.ll:35
+  %t25 = add i32 %t22, undef
+  %t26 = add i32 %t25, undef
+  %t27 = add i32 %t26, undef
----------------
Is this chain needed? also the test is probably more robust if you use concrete values instead of undef. Same for the loop condition.


================
Comment at: llvm/test/Transforms/LoopVectorize/reduction-order.ll:46
+
+!0 = !{!"clang version google3-trunk (fe5f233a938f5bc31c458c39cca54d7dcc2667ef)"}
+!1 = !{!"function_entry_count", i64 801}
----------------
Metadata not needed?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D73490