Skip to content

Conversation

@RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Apr 16, 2025

No need to explicitly check with BuildVectorSDNode::isConstant - FoldConstantArithmetic can handle this, including through bitcasts etc.

…n of constant build vectors

No need to explicitly check with BuildVectorSDNode::isConstant - FoldConstantArithmetic can handle this, including through bitcasts etc.
@llvmbot
Copy link
Member

llvmbot commented Apr 16, 2025

@llvm/pr-subscribers-backend-x86

Author: Simon Pilgrim (RKSimon)

Changes

No need to explicitly check with BuildVectorSDNode::isConstant - FoldConstantArithmetic can handle this, including through bitcasts etc.


Full diff: https://github.com/llvm/llvm-project/pull/136033.diff

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+4-7)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 3adeb4628eabf..024b653f78cb5 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -56529,16 +56529,13 @@ static SDValue combineGatherScatter(SDNode *N, SelectionDAG &DAG,
     if (IndexWidth > 32 && DAG.ComputeNumSignBits(Index) > (IndexWidth - 32)) {
       EVT NewVT = IndexVT.changeVectorElementType(MVT::i32);
 
-      // FIXME: We could support more than just constant vectors, but we need to
+      // FIXME: We could support more than just constant fold, but we need to
       // careful with costing. A truncate that can be optimized out would be
       // fine. Otherwise we might only want to create a truncate if it avoids a
       // split.
-      if (auto *BV = dyn_cast<BuildVectorSDNode>(Index)) {
-        if (BV->isConstant()) {
-          Index = DAG.getNode(ISD::TRUNCATE, DL, NewVT, Index);
-          return rebuildGatherScatter(GorS, Index, Base, Scale, DAG);
-        }
-      }
+      if (SDValue TruncIndex =
+              DAG.FoldConstantArithmetic(ISD::TRUNCATE, DL, NewVT, Index))
+        return rebuildGatherScatter(GorS, TruncIndex, Base, Scale, DAG);
 
       // Shrink any sign/zero extends from 32 or smaller to larger than 32 if
       // there are sufficient sign bits. Only do this before legalize types to

@RKSimon RKSimon merged commit f972985 into llvm:main Apr 17, 2025
8 of 11 checks passed
@RKSimon RKSimon deleted the x86-gather-scatter-index-constant-fold branch April 17, 2025 17:40
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 17, 2025

LLVM Buildbot has detected a new failure on builder arc-builder running on arc-worker while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/14683

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
0.825 [60/18/1] Linking CXX executable tools/clang/unittests/Basic/BasicTests
0.888 [59/18/2] Linking CXX executable tools/clang/unittests/Parse/ParseTests
1.662 [58/18/3] Linking CXX executable tools/clang/unittests/Format/FormatTests
5.670 [57/18/4] Linking CXX executable tools/clang/unittests/Lex/LexTests
6.152 [56/18/5] Linking CXX executable tools/clang/unittests/libclang/libclangTests
10.206 [55/18/6] Linking CXX executable tools/clang/unittests/Analysis/ClangAnalysisTests
10.615 [54/18/7] Linking CXX executable tools/clang/unittests/AST/ByteCode/InterpTests
command timed out: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1211.386719

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…n of constant build vectors (llvm#136033)

No need to explicitly check with BuildVectorSDNode::isConstant - FoldConstantArithmetic can handle this, including through bitcasts etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants