Skip to content

[RISCV] Refactor helper in isDesirableToCommuteWithShift. NFC #119526

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

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented Dec 11, 2024

Instead of duplicating the loop twice, add arguments to the lambda.
I plan on reusing this in a following patch.

@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Luke Lau (lukel97)

Changes

Instead of duplicating the loop twice, add arguments to the lambda.
I plan on reusing this in a following patch.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+10-30)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index c6838573637202..64168816420597 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -18237,41 +18237,21 @@ bool RISCVTargetLowering::isDesirableToCommuteWithShift(
   // LD/ST will optimize constant Offset extraction, so when AddNode is used by
   // LD/ST, it can still complete the folding optimization operation performed
   // above.
-  auto isUsedByLdSt = [&]() {
-    bool CanOptAlways = false;
-    if (N0->getOpcode() == ISD::ADD && !N0->hasOneUse()) {
-      for (SDNode *Use : N0->uses()) {
-        // This use is the one we're on right now. Skip it
-        if (Use == N || Use->getOpcode() == ISD::SELECT)
-          continue;
-        if (!isa<StoreSDNode>(Use) && !isa<LoadSDNode>(Use)) {
-          CanOptAlways = false;
-          break;
-        }
-        CanOptAlways = true;
-      }
-    }
-
-    if (N0->getOpcode() == ISD::SIGN_EXTEND &&
-        !N0->getOperand(0)->hasOneUse()) {
-      for (SDNode *Use : N0->getOperand(0)->uses()) {
-        // This use is the one we're on right now. Skip it
-        if (Use == N0.getNode() || Use->getOpcode() == ISD::SELECT)
-          continue;
-        if (!isa<StoreSDNode>(Use) && !isa<LoadSDNode>(Use)) {
-          CanOptAlways = false;
-          break;
-        }
-        CanOptAlways = true;
-      }
+  auto isUsedByLdSt = [](SDValue X, const SDNode *User) {
+    for (SDNode *Use : X->uses()) {
+      // This use is the one we're on right now. Skip it
+      if (Use == User || Use->getOpcode() == ISD::SELECT)
+        continue;
+      if (!isa<StoreSDNode>(Use) && !isa<LoadSDNode>(Use))
+        return false;
     }
-    return CanOptAlways;
+    return true;
   };
 
   if (Ty.isScalarInteger() &&
       (N0.getOpcode() == ISD::ADD || N0.getOpcode() == ISD::OR)) {
     if (N0.getOpcode() == ISD::ADD && !N0->hasOneUse())
-      return isUsedByLdSt();
+      return isUsedByLdSt(N0, N);
 
     auto *C1 = dyn_cast<ConstantSDNode>(N0->getOperand(1));
     auto *C2 = dyn_cast<ConstantSDNode>(N->getOperand(1));
@@ -18314,7 +18294,7 @@ bool RISCVTargetLowering::isDesirableToCommuteWithShift(
   if (N0->getOpcode() == ISD::SIGN_EXTEND &&
       N0->getOperand(0)->getOpcode() == ISD::ADD &&
       !N0->getOperand(0)->hasOneUse())
-    return isUsedByLdSt();
+    return isUsedByLdSt(N0->getOperand(0), N0.getNode());
 
   return true;
 }

Instead of duplicating the loop twice, add arguments to the lambda.
I plan on reusing this in a following patch.
@lukel97 lukel97 force-pushed the isDesirableToCommuteWithShift-isUsedByLdSt-refactor branch from e6fb3f6 to bda048a Compare December 11, 2024 08:48
lukel97 added a commit to lukel97/llvm-project that referenced this pull request Dec 11, 2024
Stacked on llvm#119526

This fixes a regression from llvm#101294 by checking if we might be clobbering a sh{1,2,3}add pattern.

Only do this is the underlying add isn't going to be folded away into an address offset.
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@lukel97 lukel97 merged commit 2698fc6 into llvm:main Dec 12, 2024
8 checks passed
lukel97 added a commit to lukel97/llvm-project that referenced this pull request Dec 12, 2024
Stacked on llvm#119526

This fixes a regression from llvm#101294 by checking if we might be clobbering a sh{1,2,3}add pattern.

Only do this is the underlying add isn't going to be folded away into an address offset.
lukel97 added a commit to lukel97/llvm-project that referenced this pull request Jan 6, 2025
Stacked on llvm#119526

This fixes a regression from llvm#101294 by checking if we might be clobbering a sh{1,2,3}add pattern.

Only do this is the underlying add isn't going to be folded away into an address offset.
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