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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

In D113300#3112717 <https://reviews.llvm.org/D113300#3112717>, @justice_adams wrote:

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

No worries - I should have pinged you, but this only came about today because I'm trying to extend constant folding to handle cases where it needs to peek through bitcasts (beyond what I've done in D113202 <https://reviews.llvm.org/D113202>), and getting this dealt with first made sense.



================
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 ||
----------------
justice_adams wrote:
> 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?
As I said in the summary - FoldConstantArithmetic doesn't bail out for non-constant source data, meaning we pick up a few identity (sub x, x) and undef style cases that we missed if we only ever called FoldConstantVectprArithmetic. 

We still bail if the result isn't constant/undef of course.


================
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
----------------
lebedev.ri wrote:
> justice_adams wrote:
> > 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.
> See first line of the file:
> ```
> ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
> ```
Yes, I just ran the update_llc_test_checks.py script - its been a massive time saver since we got those scripts, and has meant that we have much more complete checks, which has helped prevent a lot of regressions in code that we weren't (manually) checking properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113300