Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[X86] Use getAllOnesConstant instead of getConstant(-1). NFC #114299

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Oct 30, 2024

A while back I had tried to clean up cases where getConstant was implicitly truncating constants. Initially by manual audit and later with an assert. I only committed patches from the assert testing.

This code path isn't executed in lit tests so I only found it with the manual audit. This patch was still sitting in my stash so thought I go ahead and post it.

A while back I had tried to cleanup cases where getConstant was
implicitly truncating constants. Initially by manual audit and later
with an assert. I only committed patches from the assert testing.

This code path isn't executed in lit tests so I only found it with
the manual audit. I found this patch still setting in my stash so
thought I go ahead and post it.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 30, 2024

@llvm/pr-subscribers-backend-x86

Author: Craig Topper (topperc)

Changes

A while back I had tried to clean up cases where getConstant was implicitly truncating constants. Initially by manual audit and later with an assert. I only committed patches from the assert testing.

This code path isn't executed in lit tests so I only found it with the manual audit. This patch was still sitting in my stash so thought I go ahead and post it.


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

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+1-1)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 34bc5d76c15cea..22cba69af41f51 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -49321,7 +49321,7 @@ static SDValue combineVectorShiftImm(SDNode *N, SelectionDAG &DAG,
   if (!LogicalShift && ISD::isBuildVectorAllOnes(N0.getNode()))
     // N0 is all ones or undef. We guarantee that the bits shifted into the
     // result are all ones, not undef.
-    return DAG.getConstant(-1, SDLoc(N), VT);
+    return DAG.getAllOnesConstant(SDLoc(N), VT);
 
   auto MergeShifts = [&](SDValue X, uint64_t Amt0, uint64_t Amt1) {
     unsigned NewShiftVal = Amt0 + Amt1;

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@topperc topperc merged commit 847f4ef into llvm:main Oct 30, 2024
7 of 9 checks passed
@topperc topperc deleted the pr/allones branch October 30, 2024 23:22
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 31, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building llvm at step 10 "Add check check-offload".

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

Here is the relevant piece of the build log for the reference
Step 10 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug50022.cpp (866 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug53727.cpp (867 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug47654.cpp (868 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/test_libc.cpp (869 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/wtime.c (870 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/std_complex_arithmetic.cpp (871 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/bug49021.cpp (872 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/complex_reduction.cpp (873 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49021.cpp (874 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (875 of 879)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1237.864657

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.

4 participants