Skip to content

Conversation

@rj-jesus
Copy link
Contributor

@rj-jesus rj-jesus commented Aug 5, 2025

As a follow-up to #151177, when lowering SELECT_CC nodes of absolute difference patterns, drop poison-generating flags from the negated operand to avoid inadvertently propagating poison.

As discussed in #151177 (comment), I didn't find practical issues with the current code, but it seems safer to drop flags preemptively.

I don't know of a good way to test this other than by matching SDAG logs directly.

When lowering SELECT_CC nodes of absolute difference patterns, drop
poison generating flags from the negated operand to avoid inadvertently
propagating poison.
@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Ricardo Jesus (rj-jesus)

Changes

As a follow-up to #151177, when lowering SELECT_CC nodes of absolute difference patterns, drop poison-generating flags from the negated operand to avoid inadvertently propagating poison.

As discussed in #151177 (comment), I didn't find practical issues with the current code, but it seems safer to drop flags preemptively.

I don't know of a good way to test this other than by matching SDAG logs directly.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+8-3)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 4f6e3ddd18def..c1cff5b8431a8 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -11398,13 +11398,18 @@ SDValue AArch64TargetLowering::LowerSELECT_CC(
     //   select_cc lhs, rhs, sub(rhs, lhs), sub(lhs, rhs), cc ->
     //   select_cc lhs, rhs, neg(sub(lhs, rhs)), sub(lhs, rhs), cc
     // The second forms can be matched into subs+cneg.
+    // NOTE: Drop poison generating flags from the negated operand to avoid
+    // inadvertently propagating poison after the canonicalisation.
     if (TVal.getOpcode() == ISD::SUB && FVal.getOpcode() == ISD::SUB) {
       if (TVal.getOperand(0) == LHS && TVal.getOperand(1) == RHS &&
-          FVal.getOperand(0) == RHS && FVal.getOperand(1) == LHS)
+          FVal.getOperand(0) == RHS && FVal.getOperand(1) == LHS) {
         FVal = DAG.getNegative(TVal, DL, TVal.getValueType());
-      else if (TVal.getOperand(0) == RHS && TVal.getOperand(1) == LHS &&
-               FVal.getOperand(0) == LHS && FVal.getOperand(1) == RHS)
+        TVal->dropFlags(SDNodeFlags::PoisonGeneratingFlags);
+      } else if (TVal.getOperand(0) == RHS && TVal.getOperand(1) == LHS &&
+                 FVal.getOperand(0) == LHS && FVal.getOperand(1) == RHS) {
         TVal = DAG.getNegative(FVal, DL, FVal.getValueType());
+        FVal->dropFlags(SDNodeFlags::PoisonGeneratingFlags);
+      }
     }
 
     unsigned Opcode = AArch64ISD::CSEL;

@rj-jesus rj-jesus merged commit df34eac into llvm:main Aug 6, 2025
9 checks passed
@rj-jesus rj-jesus deleted the rjj/aarch64-fix-select-cc-lowering branch August 6, 2025 08:27
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