Skip to content

Conversation

woruyu
Copy link
Member

@woruyu woruyu commented Jul 18, 2025

Summary

This PR resolves #147179

@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2025

@llvm/pr-subscribers-backend-arm

Author: woruyu (woruyu)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+25)
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index fd3b0525c1056..e9ec35fa1dd8c 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -20106,6 +20106,31 @@ void ARMTargetLowering::computeKnownBitsForTargetNode(const SDValue Op,
     Known = KnownOp0.intersectWith(KnownOp1);
     break;
   }
+  case ARMISD::VORRIMM: {
+    KnownBits KnownLHS = DAG.computeKnownBits(Op.getOperand(0), Depth + 1);
+
+    unsigned Encoded = Op.getConstantOperandVal(1);
+    unsigned ElemSize = Op.getValueType().getScalarSizeInBits();
+    uint64_t DecodedVal = ARM_AM::decodeVMOVModImm(Encoded, ElemSize);
+    APInt Imm(Known.getBitWidth(), DecodedVal);
+
+    Known.One = KnownLHS.One | Imm;
+    Known.Zero = KnownLHS.Zero & ~Imm;
+    return;
+  }
+  case ARMISD::VBICIMM: {
+    KnownBits KnownLHS = DAG.computeKnownBits(Op.getOperand(0), Depth + 1);
+
+    unsigned Encoded = Op.getConstantOperandVal(1);
+    unsigned ElemSize = Op.getValueType().getScalarSizeInBits();
+    uint64_t DecodedVal = ARM_AM::decodeVMOVModImm(Encoded, ElemSize);
+    APInt Imm(Known.getBitWidth(), DecodedVal);
+
+    APInt NotImm = ~Imm;
+    Known.One = KnownLHS.One & NotImm;
+    Known.Zero = KnownLHS.Zero | Imm;
+    return;
+  }
   }
 }
 

@RKSimon RKSimon requested review from ostannard and davemgreen July 21, 2025 07:23
@RKSimon
Copy link
Collaborator

RKSimon commented Jul 21, 2025

Test coverage? Some of the ARM experts may have some suggestions.

@davemgreen
Copy link
Collaborator

Finding the tests for this is often difficult. Is it worth adding a test like AArch64SelectionDAGTest for them?

@woruyu woruyu force-pushed the feat/add-vorrimm-vbicimm-for-computeKnownBitsForTargetNode branch 3 times, most recently from c4942f4 to e8e0ba2 Compare August 15, 2025 07:39
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 for doing this, it looks good.

Copy link

github-actions bot commented Aug 18, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@woruyu woruyu force-pushed the feat/add-vorrimm-vbicimm-for-computeKnownBitsForTargetNode branch from a3722f0 to 6882840 Compare August 18, 2025 12:09
@woruyu woruyu force-pushed the feat/add-vorrimm-vbicimm-for-computeKnownBitsForTargetNode branch from 4f60b47 to f3b5b90 Compare August 29, 2025 02:56
uint64_t DecodedVal = ARM_AM::decodeVMOVModImm(Encoded, DecEltBits);
bool IsVORR = Op.getOpcode() == ARMISD::VORRIMM;

if (Op.getScalarValueSizeInBits() == DecEltBits) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please can you add a comment here - I don't know much about the VORRIMM/VBICIMM variants and a description of what the Op.getScalarValueSizeInBits() != DecEltBits scenario means

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.

[DAG][ARM] computeKnownBitsForTargetNode - add handling for ARMISD VORRIMM\VBICIMM nodes
4 participants