Skip to content

Conversation

shiltian
Copy link
Contributor

This can happen when xor cond, -1 is not combined.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Sep 23, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Shilei Tian (shiltian)

Changes

This can happen when xor cond, -1 is not combined.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIISelLowering.cpp (+20-3)
  • (added) llvm/test/CodeGen/AMDGPU/lower-brcond-with-xor.ll (+23)
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 4fd703c1f810e..b928f23a5201c 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -7540,17 +7540,34 @@ SDValue SITargetLowering::LowerBRCOND(SDValue BRCOND, SelectionDAG &DAG) const {
   SDNode *BR = nullptr;
   SDNode *SetCC = nullptr;
 
-  if (Intr->getOpcode() == ISD::SETCC) {
+  switch (Intr->getOpcode()) {
+  case ISD::SETCC: {
     // As long as we negate the condition everything is fine
     SetCC = Intr;
     Intr = SetCC->getOperand(0).getNode();
-
-  } else {
+    break;
+  }
+  case ISD::XOR: {
+    // Similar to SETCC, if we have (xor c, -1) or (xor -1, c), we will be fine.
+    SDValue LHS = Intr->getOperand(0);
+    SDValue RHS = Intr->getOperand(1);
+    if (auto *C = dyn_cast<ConstantSDNode>(LHS); C && C->getZExtValue()) {
+      Intr = RHS.getNode();
+      break;
+    }
+    if (auto *C = dyn_cast<ConstantSDNode>(RHS); C && C->getZExtValue()) {
+      Intr = LHS.getNode();
+      break;
+    }
+    [[fallthrough]];
+  }
+  default: {
     // Get the target from BR if we don't negate the condition
     BR = findUser(BRCOND, ISD::BR);
     assert(BR && "brcond missing unconditional branch user");
     Target = BR->getOperand(1);
   }
+  }
 
   unsigned CFNode = isCFIntrinsic(Intr);
   if (CFNode == 0) {
diff --git a/llvm/test/CodeGen/AMDGPU/lower-brcond-with-xor.ll b/llvm/test/CodeGen/AMDGPU/lower-brcond-with-xor.ll
new file mode 100644
index 0000000000000..e2f8df0448f82
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/lower-brcond-with-xor.ll
@@ -0,0 +1,23 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a --debug-counter=dagcombine=0 -start-before=si-annotate-control-flow %s -o - | FileCheck %s
+
+define amdgpu_kernel void @test(i32 %N, ptr addrspace(1) %p) {
+; CHECK-LABEL: test:
+; CHECK:       ; %bb.0: ; %entry
+; CHECK-NEXT:    v_and_b32_e32 v0, 0x3ff, v0
+; CHECK-NEXT:    v_cmp_gt_i32_e32 vcc, 1, v0
+; CHECK-NEXT:    s_and_saveexec_b64 s[0:1], vcc
+; CHECK-NEXT:    s_endpgm
+entry:
+  %id.x = tail call i32 @llvm.amdgcn.workitem.id.x()
+  %cmp2 = icmp slt i32 %id.x, 1
+  br i1 %cmp2, label %if.then, label %exit
+
+if.then:
+  %idx.ext = zext i32 %N to i64
+  %add.ptr = getelementptr i8, ptr addrspace(1) %p, i64 %idx.ext
+  ret void
+
+exit:
+  ret void
+}

@shiltian shiltian force-pushed the users/shiltian/lower-brcond-with-xor branch from a51379f to 8aa4395 Compare September 23, 2025 18:48
@shiltian shiltian merged commit 70a9e76 into main Sep 23, 2025
9 checks passed
@shiltian shiltian deleted the users/shiltian/lower-brcond-with-xor branch September 23, 2025 21:16
@@ -0,0 +1,23 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a --debug-counter=dagcombine=0 -start-before=si-annotate-control-flow %s -o - | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this require asserts? Does optnone work as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't seem like that, since I didn't receive any emails about test failure. Lol.

optnone doesn't work as well, because even with optnone, some combines can still happen, including this one.

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