[PATCH] D103015: [NFC] Add CHECK lines for unordered FP reductions

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

Hi @kmclaughlin I think the latest patch looks good now apart from a few minor comments about possibly confusing variable names in a couple of tests!



================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll:137
+; CHECK-UNORDERED: vector.ph
+; CHECK-UNORDERED: %[[INS_ELT1:.*]] = insertelement <vscale x 4 x float> shufflevector (<vscale x 4 x float> insertelement (<vscale x 4 x float> undef, float -0.000000e+00, i32 0), <vscale x 4 x float> undef, <vscale x 4 x i32> zeroinitializer), float %[[LOAD2]], i32 0
+; CHECK-UNORDERED: %[[INS_ELT2:.*]] = insertelement <vscale x 4 x float> shufflevector (<vscale x 4 x float> insertelement (<vscale x 4 x float> undef, float -0.000000e+00, i32 0), <vscale x 4 x float> undef, <vscale x 4 x i32> zeroinitializer), float %[[LOAD1]], i32 0
----------------
nit: Maybe this should be named INS_ELT2 to match LOAD2?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll:144
+; CHECK-UNORDERED: vector.body
+; CHECK-UNORDERED: %[[VEC_PHI1:.*]] = phi <vscale x 4 x float> [ %[[INS_ELT1]], %vector.ph ], [ %[[VEC_FADD2:.*]], %vector.body ]
+; CHECK-UNORDERED: %[[VEC_PHI2:.*]] = phi <vscale x 4 x float> [ %[[INS_ELT2]], %vector.ph ], [ %[[VEC_FADD1:.*]], %vector.body ]
----------------
nit: Again, I think maybe here it looks better as `VEC_PHI2` and `INS_ELT2` to match the second load/reduction variable? And similarly for the PHI below?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/strict-fadd.ll:218
+; CHECK-UNORDERED: vector.ph
+; CHECK-UNORDERED: %[[INS1:.*]] = insertelement <4 x float> <float -0.000000e+00, float -0.000000e+00, float -0.000000e+00, float -0.000000e+00>, float %[[LOADA2]], i32 0
+; CHECK-UNORDERED: %[[INS2:.*]] = insertelement <4 x float> <float -0.000000e+00, float -0.000000e+00, float -0.000000e+00, float -0.000000e+00>, float %[[LOADA1]], i32 0
----------------
nit: I think there is a similar naming scheme issue to the scalable equivalent?


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

https://reviews.llvm.org/D103015