Skip to content

Conversation

@davemgreen
Copy link
Collaborator

This is a fix for the second half of #75822, where smaller constants can also be bitcast to larger types. We should be checking the size is what we expect it to be when matching ones.

This is a fix for the second half of llvm#75822, where smaller constants can also
be bitcast to larger types. We should be checking the size is what we expect it
to be when matching ones.
@llvmbot
Copy link
Member

llvmbot commented Dec 27, 2023

@llvm/pr-subscribers-backend-aarch64

Author: David Green (davemgreen)

Changes

This is a fix for the second half of #75822, where smaller constants can also be bitcast to larger types. We should be checking the size is what we expect it to be when matching ones.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+4-5)
  • (modified) llvm/test/CodeGen/AArch64/neon-compare-instructions.ll (+2-1)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index dffe69bdb900db..50658a855cfb37 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -13715,11 +13715,10 @@ static SDValue EmitVectorComparison(SDValue LHS, SDValue RHS,
   bool IsCnst = BVN && BVN->isConstantSplat(SplatValue, SplatUndef,
                                             SplatBitSize, HasAnyUndefs);
 
-  bool IsSplatUniform =
-      SrcVT.getVectorElementType().getSizeInBits() >= SplatBitSize;
-  bool IsZero = IsCnst && SplatValue == 0 && IsSplatUniform;
-  bool IsOne = IsCnst && SplatValue == 1 && IsSplatUniform;
-  bool IsMinusOne = IsCnst && SplatValue.isAllOnes() && IsSplatUniform;
+  bool IsZero = IsCnst && SplatValue == 0;
+  bool IsOne =
+      IsCnst && SrcVT.getScalarSizeInBits() == SplatBitSize && SplatValue == 1;
+  bool IsMinusOne = IsCnst && SplatValue.isAllOnes();
 
   if (SrcVT.getVectorElementType().isFloatingPoint()) {
     switch (CC) {
diff --git a/llvm/test/CodeGen/AArch64/neon-compare-instructions.ll b/llvm/test/CodeGen/AArch64/neon-compare-instructions.ll
index b2fc477d8655a4..3627c670429a03 100644
--- a/llvm/test/CodeGen/AArch64/neon-compare-instructions.ll
+++ b/llvm/test/CodeGen/AArch64/neon-compare-instructions.ll
@@ -1792,7 +1792,8 @@ define <8 x i1> @not_cmle8xi8(<8 x i8> %0) {
 define <4 x i1> @not_cmle16xi8(<4 x i32> %0) {
 ; CHECK-SD-LABEL: not_cmle16xi8:
 ; CHECK-SD:       // %bb.0: // %entry
-; CHECK-SD-NEXT:    cmle v0.4s, v0.4s, #0
+; CHECK-SD-NEXT:    movi v1.8h, #1
+; CHECK-SD-NEXT:    cmgt v0.4s, v1.4s, v0.4s
 ; CHECK-SD-NEXT:    xtn v0.4h, v0.4s
 ; CHECK-SD-NEXT:    ret
 ;

Copy link
Contributor

@antoniofrighetto antoniofrighetto left a comment

Choose a reason for hiding this comment

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

Thank you for the patch, restricting the check to determine whether the splat matches the exact input size looks definitely more appropriate.

@davemgreen
Copy link
Collaborator Author

Cheers

@davemgreen davemgreen merged commit d714be9 into llvm:main Jan 2, 2024
@davemgreen davemgreen deleted the gh-a64-cmlefix branch January 2, 2024 09:24
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