[BOLT][BTI] Fix assertions checking getNumOperands#174600
Conversation
|
@llvm/pr-subscribers-bolt Author: Gergely Bálint (bgergely0) ChangesUse MCPlus::getNumPrimeOperands to check number of non-annotation Full diff: https://github.com/llvm/llvm-project/pull/174600.diff 1 Files Affected:
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index 03fb4ddc2f238..a30799d5f45d3 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -2818,7 +2818,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
// x16 or x17. If the operand is not x16 or x17, it can be accepted by a BTI
// j or BTI jc (and not BTI c).
if (isIndirectBranch(Call)) {
- assert(Call.getNumOperands() == 1 &&
+ assert(MCPlus::getNumPrimeOperands(Call) == 1 &&
"Indirect branch needs to have 1 operand.");
assert(Call.getOperand(0).isReg() &&
"Indirect branch does not have a register operand.");
@@ -2856,7 +2856,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
// x16 or x17. If the operand is not x16 or x17, it can be accepted by a
// BTI j or BTI jc (and not BTI c).
if (isIndirectBranch(Call)) {
- assert(Call.getNumOperands() == 1 &&
+ assert(MCPlus::getNumPrimeOperands(Call) == 1 &&
"Indirect branch needs to have 1 operand.");
assert(Call.getOperand(0).isReg() &&
"Indirect branch does not have a register operand.");
|
peterwaller-arm
left a comment
There was a problem hiding this comment.
Fix sounds reasonable though it makes me wonder if other uses of getNumOperands are wrong: what about the IsExplicitBTI definition, for example?
that's a good point. Let me check other uses, and add a few relevant tests. |
Several BTI-related functions are checking that a call MCInst has one non-annotation operand. This patch changes these checks to use MCPlus::getNumPrimeOperands, instead of getNumOperands.
15dceca to
da54184
Compare
paschalis-mpeis
left a comment
There was a problem hiding this comment.
Looks good! Thanks Gergely, can you update the commit message to note that unit tests were updated to cover this?

Several BTI-related functions are checking that a call MCInst has one
non-annotation operand.
This patch changes these checks to use MCPlus::getNumPrimeOperands,
instead of getNumOperands.
Testing: added annotations to existing gtests to serve as regression tests.
These now also explicitly check getNumOperands and getNumPrimeOperands
usage on the annotated MCInsts.