-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[DAG][ARM] computeKnownBitsForTargetNode - add handling for ARMISD VORRIMM\VBICIMM nodes #149494
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
[DAG][ARM] computeKnownBitsForTargetNode - add handling for ARMISD VORRIMM\VBICIMM nodes #149494
Conversation
@llvm/pr-subscribers-backend-arm Author: woruyu (woruyu) ChangesFull diff: https://github.com/llvm/llvm-project/pull/149494.diff 1 Files Affected:
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;
+ }
}
}
|
Test coverage? Some of the ARM experts may have some suggestions. |
Finding the tests for this is often difficult. Is it worth adding a test like AArch64SelectionDAGTest for them? |
c4942f4
to
e8e0ba2
Compare
There was a problem hiding this 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.
✅ With the latest revision this PR passed the C/C++ code formatter. |
a3722f0
to
6882840
Compare
4f60b47
to
f3b5b90
Compare
…RRIMM\VBICIMM nodes
f3b5b90
to
a7fbc84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - but will leave it to an ARM expert to accept
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM.
Are you happy for this to be submitted?
Yes, thanks. I’ll land this shortly. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/26690 Here is the relevant piece of the build log for the reference
|
I looked into the This isn’t caused by my patch. The builder is hitting an environment issue:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/12646 Here is the relevant piece of the build log for the reference
|
The clang-ppc64le-linux-multistage failure does look related though - missing Analysis lib dependency:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/9555 Here is the relevant piece of the build log for the reference
|
clang-ppc64le-rhel appears to missing LLVMAsmParser
|
Summary
This PR resolves #147179