[AArch64] Enable implicit null check transformation
This change enables the generic implicit null transformation for the AArch64 target. As background for those unfamiliar with our implicit null check support:
An implicit null check is the use of a signal handler to catch and redirect to a handler a null pointer. Specifically, it's replacing an explicit conditional branch with such a redirect. This is only done for very cold branches under frontend control w/appropriate metadata.
FAULTING_OP is used to wrap the faulting instruction. It is modelled as being a conditional branch to reflect the fact it can transfer control in the CFG.
FAULTING_OP does not need to be an analyzable branch to achieve it's purpose. (Or at least, that's the x86 model. I find this slightly questionable.)
When lowering to MC, we convert the FAULTING_OP back into the actual instruction, record the labels, and lower the original instruction.
As can be seen in the test changes, currently the AArch64 backend does not eliminate the unconditional branch to the fallthrough block. I've tried two approaches, neither of which worked. I plan to return to this in a separate change set once I've wrapped my head around the interactions a bit better. (X86 handles this via AllowModify on analyzeBranch, but adding the obvious code causing BranchFolding to crash. I haven't yet figured out if it's a latent bug in BranchFolding, or something I'm doing wrong.)
Differential Revision: https://reviews.llvm.org/D87851
diff --git a/llvm/lib/CodeGen/BranchRelaxation.cpp b/llvm/lib/CodeGen/BranchRelaxation.cpp
index 5a3ec1a..366c303 100644
--- a/llvm/lib/CodeGen/BranchRelaxation.cpp
+++ b/llvm/lib/CodeGen/BranchRelaxation.cpp
@@ -507,25 +507,31 @@
Next = std::next(J);
MachineInstr &MI = *J;
- if (MI.isConditionalBranch()) {
- MachineBasicBlock *DestBB = TII->getBranchDestBlock(MI);
- if (!isBlockInRange(MI, *DestBB)) {
- if (Next != MBB.end() && Next->isConditionalBranch()) {
- // If there are multiple conditional branches, this isn't an
- // analyzable block. Split later terminators into a new block so
- // each one will be analyzable.
+ if (!MI.isConditionalBranch())
+ continue;
- splitBlockBeforeInstr(*Next, DestBB);
- } else {
- fixupConditionalBranch(MI);
- ++NumConditionalRelaxed;
- }
+ if (MI.getOpcode() == TargetOpcode::FAULTING_OP)
+ // FAULTING_OP's destination is not encoded in the instruction stream
+ // and thus never needs relaxed.
+ continue;
- Changed = true;
+ MachineBasicBlock *DestBB = TII->getBranchDestBlock(MI);
+ if (!isBlockInRange(MI, *DestBB)) {
+ if (Next != MBB.end() && Next->isConditionalBranch()) {
+ // If there are multiple conditional branches, this isn't an
+ // analyzable block. Split later terminators into a new block so
+ // each one will be analyzable.
- // This may have modified all of the terminators, so start over.
- Next = MBB.getFirstTerminator();
+ splitBlockBeforeInstr(*Next, DestBB);
+ } else {
+ fixupConditionalBranch(MI);
+ ++NumConditionalRelaxed;
}
+
+ Changed = true;
+
+ // This may have modified all of the terminators, so start over.
+ Next = MBB.getFirstTerminator();
}
}
}
diff --git a/llvm/lib/CodeGen/ImplicitNullChecks.cpp b/llvm/lib/CodeGen/ImplicitNullChecks.cpp
index 9030f32..c2b764e 100644
--- a/llvm/lib/CodeGen/ImplicitNullChecks.cpp
+++ b/llvm/lib/CodeGen/ImplicitNullChecks.cpp
@@ -373,10 +373,14 @@
bool OffsetIsScalable;
const MachineOperand *BaseOp;
+ // Implementation restriction for faulting_op insertion
+ // TODO: This could be relaxed if we find a test case which warrants it.
+ if (MI.getDesc().getNumDefs() > 1)
+ return SR_Unsuitable;
// FIXME: This handles only simple addressing mode.
if (!TII->getMemOperandWithOffset(MI, BaseOp, Offset, OffsetIsScalable, TRI))
- return SR_Unsuitable;
+ return SR_Unsuitable;
// We need the base of the memory instruction to be same as the register
// where the null check is performed (i.e. PointerReg).
@@ -502,9 +506,9 @@
MBP.Predicate == MachineBranchPredicate::PRED_EQ)))
return false;
- // If we cannot erase the test instruction itself, then making the null check
- // implicit does not buy us much.
- if (!MBP.SingleUseCondition)
+ // If there is a separate condition generation instruction, we chose not to
+ // transform unless we can remove both condition and consuming branch.
+ if (MBP.ConditionDef && !MBP.SingleUseCondition)
return false;
MachineBasicBlock *NotNullSucc, *NullSucc;
@@ -522,32 +526,34 @@
if (NotNullSucc->pred_size() != 1)
return false;
- // To prevent the invalid transformation of the following code:
- //
- // mov %rax, %rcx
- // test %rax, %rax
- // %rax = ...
- // je throw_npe
- // mov(%rcx), %r9
- // mov(%rax), %r10
- //
- // into:
- //
- // mov %rax, %rcx
- // %rax = ....
- // faulting_load_op("movl (%rax), %r10", throw_npe)
- // mov(%rcx), %r9
- //
- // we must ensure that there are no instructions between the 'test' and
- // conditional jump that modify %rax.
const Register PointerReg = MBP.LHS.getReg();
- assert(MBP.ConditionDef->getParent() == &MBB && "Should be in basic block");
+ if (MBP.ConditionDef) {
+ // To prevent the invalid transformation of the following code:
+ //
+ // mov %rax, %rcx
+ // test %rax, %rax
+ // %rax = ...
+ // je throw_npe
+ // mov(%rcx), %r9
+ // mov(%rax), %r10
+ //
+ // into:
+ //
+ // mov %rax, %rcx
+ // %rax = ....
+ // faulting_load_op("movl (%rax), %r10", throw_npe)
+ // mov(%rcx), %r9
+ //
+ // we must ensure that there are no instructions between the 'test' and
+ // conditional jump that modify %rax.
+ assert(MBP.ConditionDef->getParent() == &MBB &&
+ "Should be in basic block");
- for (auto I = MBB.rbegin(); MBP.ConditionDef != &*I; ++I)
- if (I->modifiesRegister(PointerReg, TRI))
- return false;
-
+ for (auto I = MBB.rbegin(); MBP.ConditionDef != &*I; ++I)
+ if (I->modifiesRegister(PointerReg, TRI))
+ return false;
+ }
// Starting with a code fragment like:
//
// test %rax, %rax
@@ -726,9 +732,11 @@
}
NC.getMemOperation()->eraseFromParent();
- NC.getCheckOperation()->eraseFromParent();
+ if (auto *CheckOp = NC.getCheckOperation())
+ CheckOp->eraseFromParent();
- // Insert an *unconditional* branch to not-null successor.
+ // Insert an *unconditional* branch to not-null successor - we expect
+ // block placement to remove fallthroughs later.
TII->insertBranch(*NC.getCheckBlock(), NC.getNotNullSucc(), nullptr,
/*Cond=*/None, DL);