Skip to content

[AArch64] Fix copy and paste error in tryCombineMULLWithUZP1() (NFCI) #97729

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

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jul 4, 2024

The bitcast check was performed on the wrong value in one of the branches.

I believe this doesn't actually result in any behavior difference, because the following code looking at ExtractHigh users currently doesn't try to look through BITCASTS. I've left a TODO for that.

Fixes #94761.

The bitcast check was performed on the wrong value in one of the
branches.

I believe this doesn't actually result in any behavior difference,
because the following code looking at ExtractHigh users doesn't
try to look through BITCASTS. I've left a TODO for that.

Fixes llvm#94761.
@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2024

@llvm/pr-subscribers-backend-aarch64

Author: Nikita Popov (nikic)

Changes

The bitcast check was performed on the wrong value in one of the branches.

I believe this doesn't actually result in any behavior difference, because the following code looking at ExtractHigh users currently doesn't try to look through BITCASTS. I've left a TODO for that.

Fixes #94761.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+2-1)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index e0c3cc5eddb82..760ae32d1059a 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -24829,7 +24829,7 @@ static SDValue tryCombineMULLWithUZP1(SDNode *N,
   } else if (isEssentiallyExtractHighSubvector(RHS) &&
              LHS.getOpcode() == ISD::TRUNCATE) {
     TruncHigh = LHS;
-    if (LHS.getOpcode() == ISD::BITCAST)
+    if (RHS.getOpcode() == ISD::BITCAST)
       ExtractHigh = RHS.getOperand(0);
     else
       ExtractHigh = RHS;
@@ -24858,6 +24858,7 @@ static SDValue tryCombineMULLWithUZP1(SDNode *N,
   // This dagcombine assumes the two extract_high uses same source vector in
   // order to detect the pair of the mull. If they have different source vector,
   // this code will not work.
+  // TODO: Should also try to look through a bitcast.
   bool HasFoundMULLow = true;
   SDValue ExtractHighSrcVec = ExtractHigh.getOperand(0);
   if (ExtractHighSrcVec->use_size() != 2)

@davemgreen davemgreen requested a review from jaykang10 July 5, 2024 09:58
Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

Thanks, I think I agree that this should be an NFC. LGTM

@nikic nikic merged commit 1604c24 into llvm:main Jul 8, 2024
9 checks passed
@nikic nikic deleted the aarch64-mull-fix branch July 8, 2024 07:42
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.

[AArch64] Copy and paste error in tryCombineMULLWithUZP1() fold
3 participants