[PATCH] D113300: [SelectionDAG] Merge FoldConstantVectorArithmetic into FoldConstantArithmetic (PR36544)

[Justice Adams via Phabricator via llvm-commits]

justice_adams added a comment.

Hi @RKSimon , 
First, let me apologize as I completely forgot the original bug was assigned to me. Thank you for tackling this.

This LGTM (pending the fix to the DAGCombiner.cpp file) but I'm no expert so another review would be ideal. I left a couple comments that are more for my own understanding rather than actual review comments :)



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5319
 
-  auto IsConstantBuildVectorSplatVectorOrUndef = [](const SDValue &Op) {
-    APInt SplatVal;
-    BuildVectorSDNode *BV = dyn_cast<BuildVectorSDNode>(Op);
+  auto IsBuildVectorSplatVectorOrUndef = [](const SDValue &Op) {
     return Op.isUndef() || Op.getOpcode() == ISD::CONDCODE ||
----------------
Question: Are we just removing the need for the build/splat vector to be constant here because it's not needed? If so, I wonder why it was enforced in the first place?


================
Comment at: llvm/test/CodeGen/RISCV/urem-seteq-illegal-types.ll:644
+; RV64MV-NEXT:    addi a1, a1, %lo(.LCPI4_0)
+; RV64MV-NEXT:    vle16.v v10, (a1)
+; RV64MV-NEXT:    vid.v v11
----------------
Question: This is a bit of an open ended question for my own learning, but how did you know what to update these tests to? I understand the updating of the SelectionDag code, but have a hard time understanding how you figured out what to update these tests to so that they pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113300