[PATCH] D83779: [SLP] Fix order of `insertelement`/`insertvalue` seed operands

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7084
+    return true;
+  } else {
+    auto *IV = cast<InsertValueInst>(InsertInst);
----------------
No need for `else` here, just unconditional block, since `then` part is terminated with `return`


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7118
+      return true;
+    } else
+      return false;
----------------
No need for `else` here


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7120
+      return false;
+  } else {
+    auto *IV = cast<InsertValueInst>(InsertInst);
----------------
Remove `else` here


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7131
+      } else
+        return false;
+      OperandIndex += Index;
----------------
Enclose in braces


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7161
+
+  if (BuildVectorOpds.size() == 0) {
+    unsigned int AggregateSize;
----------------
`.empty()`


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7363
+  if (!findBuildAggregate(IVI, TTI, BuildVectorOpds, BuildVectorInsts, 0) ||
+      (!llvm::all_of(BuildVectorOpds, [](Value *V) { return V != nullptr; })))
     return false;
----------------
Very strange check, why we need it? Why did you change the previous one?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7378
+  if (!findBuildAggregate(IEI, TTI, BuildVectorOpds, BuildVectorInsts, 0) ||
+      (!llvm::all_of(BuildVectorOpds, [](Value *V) { return V != nullptr; })) ||
       (llvm::all_of(BuildVectorOpds,
----------------
Same question here


================
Comment at: llvm/test/Transforms/SLPVectorizer/AArch64/gather-root.ll:206-209
+; MAX-COST-NEXT:    [[P0:%.*]] = load i8, i8* getelementptr inbounds ([80 x i8], [80 x i8]* @a, i64 0, i64 1), align 1
+; MAX-COST-NEXT:    [[P1:%.*]] = icmp eq i8 [[P0]], 0
+; MAX-COST-NEXT:    [[P2:%.*]] = load i8, i8* getelementptr inbounds ([80 x i8], [80 x i8]* @a, i64 0, i64 2), align 2
+; MAX-COST-NEXT:    [[P3:%.*]] = icmp eq i8 [[P2]], 0
----------------
A regression?


================
Comment at: llvm/test/Transforms/SLPVectorizer/X86/alternate-cast.ll:293-296
+; CHECK-NEXT:    [[AB0:%.*]] = sitofp i32 [[A0]] to float
+; CHECK-NEXT:    [[AB1:%.*]] = sitofp i32 [[A1]] to float
+; CHECK-NEXT:    [[AB2:%.*]] = sitofp i32 [[A2]] to float
+; CHECK-NEXT:    [[AB3:%.*]] = sitofp i32 [[A3]] to float
----------------
Regression?


================
Comment at: llvm/test/Transforms/SLPVectorizer/X86/insert-element-build-vector.ll:129-134
+; ANY-NEXT:    [[REORDER_SHUFFLE:%.*]] = shufflevector <4 x i32> [[C:%.*]], <4 x i32> undef, <4 x i32> <i32 2, i32 1, i32 0, i32 3>
+; ANY-NEXT:    [[REORDER_SHUFFLE1:%.*]] = shufflevector <4 x float> [[A:%.*]], <4 x float> undef, <4 x i32> <i32 2, i32 1, i32 0, i32 3>
+; ANY-NEXT:    [[REORDER_SHUFFLE2:%.*]] = shufflevector <4 x float> [[B:%.*]], <4 x float> undef, <4 x i32> <i32 2, i32 1, i32 0, i32 3>
+; ANY-NEXT:    [[TMP1:%.*]] = icmp ne <4 x i32> [[REORDER_SHUFFLE]], zeroinitializer
+; ANY-NEXT:    [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x float> [[REORDER_SHUFFLE1]], <4 x float> [[REORDER_SHUFFLE2]]
+; ANY-NEXT:    [[TMP3:%.*]] = extractelement <4 x float> [[TMP2]], i32 2
----------------
Regression?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83779