[PATCH] D76947: [SelectionDAGBuilder][CGP][X86] Move some of SDB's gather/scatter uniform base handling to CGP.

[Craig Topper via Phabricator via llvm-commits]

craig.topper marked 3 inline comments as done.
craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:5242
+      C = C->getSplatValue();
+    auto *CI = dyn_cast_or_null<ConstantInt>(C);
+    if (!CI || !CI->isZero())
----------------
efriedma wrote:
> Does it matter whether the value is specifically zero, as opposed to an arbitrary splat value?
It shouldn't matter, but I was just trying to avoid changing too much relative to the existing SelectionDAGBuilder code in this patch.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:5257
+  // We need different handling for structs and sequential types.
+  if (GTI.isStruct()) {
+    // Scalarize the struct index if needed.
----------------
efriedma wrote:
> I think it would make sense to unify the handling where the last index is a scalar/splat, rather that splitting it based on whether `GTI.isStruct()` is true.
Good point. I added the scalar/splat case later after facing some regressions.


================
Comment at: llvm/lib/IR/Verifier.cpp:2886
            "Call parameter type does not match function signature!",
-           Call.getArgOperand(i), FTy->getParamType(i), Call);
+           Call.getArgOperand(i)->getType(), FTy->getParamType(i), Call);
 
----------------
efriedma wrote:
> Not sure this is an improvement.
I think this was from debugging something and I didn't mean to leave it in here. I'll remove it.


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

https://reviews.llvm.org/D76947