[AArch64] Enable implicit null check transformation

[reames]

Summary

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.)

[efriedma]

Please fix clang-format warnings. Otherwise LGTM.

llvm/lib/CodeGen/BranchRelaxation.cpp

for (MachineFunction::iterator I = MF->begin(); I != MF->end(); ++I) {
 
     // Loop over the conditional branches.
     MachineBasicBlock::iterator Next;
     for (MachineBasicBlock::iterator J = MBB.getFirstTerminator();
          J != MBB.end(); J = Next) {
       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;
+
+      if (MI.getOpcode() == TargetOpcode::FAULTING_OP)
+        // FAULTING_OP's destination is not encoded in the instruction stream
+        // and thus never needs relaxed.
+        continue;
+
 
-            splitBlockBeforeInstr(*Next, DestBB);
-          } else {
-            fixupConditionalBranch(MI);
-            ++NumConditionalRelaxed;
-          }
 
-          Changed = true;
 
-          // This may have modified all of the terminators, so start over.
-          Next = MBB.getFirstTerminator();
-        }
-      }
-    }
-  }
 
   return Changed;
 }
 
 bool BranchRelaxation::runOnMachineFunction(MachineFunction &mf) {
   MF = &mf;
 
   LLVM_DEBUG(dbgs() << "***** BranchRelaxation *****\n");

llvm/lib/CodeGen/ImplicitNullChecks.cpp

 ImplicitNullChecks::SuitabilityResult
 ImplicitNullChecks::isSuitableMemoryOp(const MachineInstr &MI,
                                        unsigned PointerReg,
                                        ArrayRef<MachineInstr *> PrevInsts) {
   int64_t Offset;
   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;
 
   // We need the base of the memory instruction to be same as the register
   // where the null check is performed (i.e. PointerReg).
   if (!BaseOp->isReg() || BaseOp->getReg() != PointerReg)
     return SR_Unsuitable;
 
   // Scalable offsets are a part of scalable vectors (SVE for AArch64). That
   // target is in-practice unsupported for ImplicitNullChecks.
if (TII->analyzeBranchPredicate(MBB, MBP, true))
     return false;
 
   // Is the predicate comparing an integer to zero?
   if (!(MBP.LHS.isReg() && MBP.RHS.isImm() && MBP.RHS.getImm() == 0 &&
         (MBP.Predicate == MachineBranchPredicate::PRED_NE ||
          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;
 
   if (MBP.Predicate == MachineBranchPredicate::PRED_NE) {
     NotNullSucc = MBP.TrueDest;
     NullSucc = MBP.FalseDest;
   } else {
     NotNullSucc = MBP.FalseDest;
     NullSucc = MBP.TrueDest;
   }
 
   // We handle the simplest case for now.  We can potentially do better by using
   // the machine dominator tree.
   if (NotNullSucc->pred_size() != 1)
     return false;
 
+  const Register PointerReg = MBP.LHS.getReg();
+
+  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.
-  const Register PointerReg = MBP.LHS.getReg();
-
-  assert(MBP.ConditionDef->getParent() ==  &MBB && "Should be in basic block");
+    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;
-
+  }
   // Starting with a code fragment like:
   //
   //   test %rax, %rax
   //   jne LblNotNull
   //
   //  LblNull:
   //   callq throw_NullPointerException
   //
if (auto *DepMI = NC.getOnlyDependency()) {
         if (!MO.isReg() || !MO.getReg() || !MO.isDef() || MO.isDead())
           continue;
         if (!NC.getNotNullSucc()->isLiveIn(MO.getReg()))
           NC.getNotNullSucc()->addLiveIn(MO.getReg());
       }
     }
 
     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);
 
     NumImplicitNullChecks++;
   }
 }
 
 char ImplicitNullChecks::ID = 0;
 
 char &llvm::ImplicitNullChecksID = ImplicitNullChecks::ID;
 
 INITIALIZE_PASS_BEGIN(ImplicitNullChecks, DEBUG_TYPE,
                       "Implicit null checks", false, false)
 INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
 INITIALIZE_PASS_END(ImplicitNullChecks, DEBUG_TYPE,
                     "Implicit null checks", false, false)

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/CodeGen/AsmPrinter.h"
+#include "llvm/CodeGen/FaultMaps.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineJumpTableInfo.h"
 #include "llvm/CodeGen/MachineModuleInfoImpls.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/StackMaps.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"

 
 #define DEBUG_TYPE "asm-printer"
 
 namespace {
 
 class AArch64AsmPrinter : public AsmPrinter {
   AArch64MCInstLower MCInstLowering;
   StackMaps SM;
+  FaultMaps FM;
   const AArch64Subtarget *STI;
 
 public:
   AArch64AsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer)
       : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(OutContext, *this),
-        SM(*this) {}
+        SM(*this), FM(*this) {}
 
   StringRef getPassName() const override { return "AArch64 Assembly Printer"; }
 
   /// Wrapper for MCInstLowering.lowerOperand() for the
   /// tblgen'erated pseudo lowering.
   bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp) const {
     return MCInstLowering.lowerOperand(MO, MCOp);
   }
 
   void emitStartOfAsmFile(Module &M) override;
   void emitJumpTableInfo() override;
   void emitJumpTableEntry(const MachineJumpTableInfo *MJTI,
                           const MachineBasicBlock *MBB, unsigned JTI);
 
   void LowerJumpTableDestSmall(MCStreamer &OutStreamer, const MachineInstr &MI);
 
   void LowerSTACKMAP(MCStreamer &OutStreamer, StackMaps &SM,
                      const MachineInstr &MI);
   void LowerPATCHPOINT(MCStreamer &OutStreamer, StackMaps &SM,
                        const MachineInstr &MI);
   void LowerSTATEPOINT(MCStreamer &OutStreamer, StackMaps &SM,
                        const MachineInstr &MI);
+  void LowerFAULTING_OP(const MachineInstr &MI);
 
   void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI);
   void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr &MI);
   void LowerPATCHABLE_TAIL_CALL(const MachineInstr &MI);
 
   typedef std::tuple<unsigned, bool, uint32_t> HwasanMemaccessTuple;
   std::map<HwasanMemaccessTuple, MCSymbol *> HwasanMemaccessSymbols;
   void LowerHWASAN_CHECK_MEMACCESS(const MachineInstr &MI);
void AArch64AsmPrinter::emitEndOfAsmFile(Module &M) {
   if (TT.isOSBinFormatMachO()) {
     // Funny Darwin hack: This flag tells the linker that no global symbols
     // contain code that falls through to other global symbols (e.g. the obvious
     // implementation of multiple entry points).  If this doesn't occur, the
     // linker can safely perform dead code stripping.  Since LLVM never
     // generates code that does this, it is always safe to set.
     OutStreamer->emitAssemblerFlag(MCAF_SubsectionsViaSymbols);
   }
+  
+  // Emit stack and fault map information.
   emitStackMaps(SM);
+  FM.serializeToFaultMapSection();
+
 }
 
 void AArch64AsmPrinter::EmitLOHs() {
   SmallVector<MCSymbol *, 3> MCArgs;
 
   for (const auto &D : AArch64FI->getLOHContainer()) {
     for (const MachineInstr *MI : D.getArgs()) {
       MInstToMCSymbol::iterator LabelIt = LOHInstToLabel.find(MI);
void AArch64AsmPrinter::LowerSTATEPOINT(MCStreamer &OutStreamer, StackMaps &SM,
   }
 
   auto &Ctx = OutStreamer.getContext();
   MCSymbol *MILabel = Ctx.createTempSymbol();
   OutStreamer.emitLabel(MILabel);
   SM.recordStatepoint(*MILabel, MI);
 }
 
+void AArch64AsmPrinter::LowerFAULTING_OP(const MachineInstr &FaultingMI) {
+  // FAULTING_LOAD_OP <def>, <faltinf type>, <MBB handler>,
+  //                  <opcode>, <operands>
+
+  Register DefRegister = FaultingMI.getOperand(0).getReg();
+  FaultMaps::FaultKind FK =
+      static_cast<FaultMaps::FaultKind>(FaultingMI.getOperand(1).getImm());
+  MCSymbol *HandlerLabel = FaultingMI.getOperand(2).getMBB()->getSymbol();
+  unsigned Opcode = FaultingMI.getOperand(3).getImm();
+  unsigned OperandsBeginIdx = 4;
+
+  auto &Ctx = OutStreamer->getContext();
+  MCSymbol *FaultingLabel = Ctx.createTempSymbol();
+  OutStreamer->emitLabel(FaultingLabel);
+
+  assert(FK < FaultMaps::FaultKindMax && "Invalid Faulting Kind!");
+  FM.recordFaultingOp(FK, FaultingLabel, HandlerLabel);
+
+  MCInst MI;
+  MI.setOpcode(Opcode);
+
+  if (DefRegister != (Register)0)
+    MI.addOperand(MCOperand::createReg(DefRegister));
+
+  for (auto I = FaultingMI.operands_begin() + OperandsBeginIdx,
+            E = FaultingMI.operands_end();
+       I != E; ++I) {
+    MCOperand Dest;
+    lowerOperand(*I, Dest);
+    MI.addOperand(Dest);
+  }
+
+  OutStreamer->AddComment("on-fault: " + HandlerLabel->getName());
+  OutStreamer->emitInstruction(MI, getSubtargetInfo());
+}
+
 void AArch64AsmPrinter::EmitFMov0(const MachineInstr &MI) {
   Register DestReg = MI.getOperand(0).getReg();
   if (STI->hasZeroCycleZeroingFP() && !STI->hasZeroCycleZeroingFPWorkaround()) {
     // Convert H/S/D register to corresponding Q register
     if (AArch64::H0 <= DestReg && DestReg <= AArch64::H31)
       DestReg = AArch64::Q0 + (DestReg - AArch64::H0);
     else if (AArch64::S0 <= DestReg && DestReg <= AArch64::S31)
       DestReg = AArch64::Q0 + (DestReg - AArch64::S0);
case TargetOpcode::STACKMAP:
     return LowerSTACKMAP(*OutStreamer, SM, *MI);
 
   case TargetOpcode::PATCHPOINT:
     return LowerPATCHPOINT(*OutStreamer, SM, *MI);
 
   case TargetOpcode::STATEPOINT:
     return LowerSTATEPOINT(*OutStreamer, SM, *MI);
 
+  case TargetOpcode::FAULTING_OP:
+    return LowerFAULTING_OP(*MI);
+
   case TargetOpcode::PATCHABLE_FUNCTION_ENTER:
     LowerPATCHABLE_FUNCTION_ENTER(*MI);
     return;
 
   case TargetOpcode::PATCHABLE_FUNCTION_EXIT:
     LowerPATCHABLE_FUNCTION_EXIT(*MI);
     return;
 

llvm/lib/Target/AArch64/AArch64InstrInfo.h

bool isBranchOffsetInRange(unsigned BranchOpc,
                              int64_t BrOffset) const override;
 
   MachineBasicBlock *getBranchDestBlock(const MachineInstr &MI) const override;
 
   bool analyzeBranch(MachineBasicBlock &MBB, MachineBasicBlock *&TBB,
                      MachineBasicBlock *&FBB,
                      SmallVectorImpl<MachineOperand> &Cond,
                      bool AllowModify = false) const override;
+  bool analyzeBranchPredicate(MachineBasicBlock &MBB,
+                              MachineBranchPredicate &MBP,
+                              bool AllowModify) const override;
   unsigned removeBranch(MachineBasicBlock &MBB,
                         int *BytesRemoved = nullptr) const override;
   unsigned insertBranch(MachineBasicBlock &MBB, MachineBasicBlock *TBB,
                         MachineBasicBlock *FBB, ArrayRef<MachineOperand> Cond,
                         const DebugLoc &DL,
                         int *BytesAdded = nullptr) const override;
   bool
   reverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const override;

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

if (AllowModify)
       I->eraseFromParent();
     return true;
   }
 
   // Otherwise, can't handle this.
   return true;
 }
 
+bool AArch64InstrInfo::analyzeBranchPredicate(MachineBasicBlock &MBB,
+                                              MachineBranchPredicate &MBP,
+                                              bool AllowModify) const {
+  // For the moment, handle only a block which ends with a cb(n)zx followed by
+  // a fallthrough.  Why this?  Because it is a common form.
+  // TODO: Should we handle b.cc?
+
+  MachineBasicBlock::iterator I = MBB.getLastNonDebugInstr();
+  if (I == MBB.end())
+    return true;
+
+  // Skip over SpeculationBarrierEndBB terminators
+  if (I->getOpcode() == AArch64::SpeculationBarrierISBDSBEndBB ||
+      I->getOpcode() == AArch64::SpeculationBarrierSBEndBB) {
+    --I;
+  }
+
+  if (!isUnpredicatedTerminator(*I))
+    return true;
+
+  // Get the last instruction in the block.
+  MachineInstr *LastInst = &*I;
+  unsigned LastOpc = LastInst->getOpcode();
+  if (!isCondBranchOpcode(LastOpc))
+    return true;
+
+  switch (LastOpc) {
+  default:
+    return true;
+  case AArch64::CBZW:
+  case AArch64::CBZX:
+  case AArch64::CBNZW:
+  case AArch64::CBNZX:
+    break;
+  };
+
+  MBP.TrueDest = LastInst->getOperand(1).getMBB();
+  assert(MBP.TrueDest && "expected!");
+  MBP.FalseDest = MBB.getNextNode();
+
+  MBP.ConditionDef = nullptr;
+  MBP.SingleUseCondition = false;
+
+  MBP.LHS = LastInst->getOperand(0);
+  MBP.RHS = MachineOperand::CreateImm(0);
+  MBP.Predicate = LastOpc == AArch64::CBNZX ? MachineBranchPredicate::PRED_NE
+                                            : MachineBranchPredicate::PRED_EQ;
+  return false;
+}
+
 bool AArch64InstrInfo::reverseBranchCondition(
     SmallVectorImpl<MachineOperand> &Cond) const {
   if (Cond[0].getImm() != -1) {
     // Regular Bcc
     AArch64CC::CondCode CC = (AArch64CC::CondCode)(int)Cond[0].getImm();
     Cond[0].setImm(AArch64CC::getInvertedCondCode(CC));
   } else {
     // Folded compare-and-branch

llvm/test/CodeGen/AArch64/implicit-null-check.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -verify-machineinstrs -O3 -mtriple=aarch64-unknown-unknown -enable-implicit-null-checks | FileCheck %s
 
 ; Basic test for implicit null check conversion - this is analogous to the
 ; file with the same name in the X86 tree, but adjusted to remove patterns
 ; related to memory folding of arithmetic (since aarch64 doesn't), and add
 ; a couple of aarch64 specific tests.
-; NOTE: Currently negative tests as these are being precommitted before
-; the changes to enable.
 
 define i32 @imp_null_check_load_fallthrough(i32* %x) {
 ; CHECK-LABEL: imp_null_check_load_fallthrough:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB0_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldr w0, [x0]
+; CHECK-NEXT:  .Ltmp0:
+; CHECK-NEXT:    ldr w0, [x0] // on-fault: .LBB0_2
+; CHECK-NEXT:    b .LBB0_1
+; CHECK-NEXT:  .LBB0_1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB0_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  not_null:
   %t = load i32, i32* %x
   ret i32 %t
 
 is_null:
   ret i32 42
 }
 
 
 define i32 @imp_null_check_load_reorder(i32* %x) {
 ; CHECK-LABEL: imp_null_check_load_reorder:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB1_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldr w0, [x0]
+; CHECK-NEXT:  .Ltmp1:
+; CHECK-NEXT:    ldr w0, [x0] // on-fault: .LBB1_2
+; CHECK-NEXT:    b .LBB1_1
+; CHECK-NEXT:  .LBB1_1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB1_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret i32 42
 
  not_null:
   %t = load i32, i32* %x
   ret i32 %t
 }
 
 define i32 @imp_null_check_unordered_load(i32* %x) {
 ; CHECK-LABEL: imp_null_check_unordered_load:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB2_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldr w0, [x0]
+; CHECK-NEXT:  .Ltmp2:
+; CHECK-NEXT:    ldr w0, [x0] // on-fault: .LBB2_2
+; CHECK-NEXT:    b .LBB2_1
+; CHECK-NEXT:  .LBB2_1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB2_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret i32 42
 
  not_null:
   %t = load atomic i32, i32* %x unordered, align 4
   ret i32 %t
 }
 
 
+; TODO: Can be converted into implicit check.
+;; Probably could be implicit, but we're conservative for now
 define i32 @imp_null_check_seq_cst_load(i32* %x) {
 ; CHECK-LABEL: imp_null_check_seq_cst_load:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    cbz x0, .LBB3_2
 ; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    ldar w0, [x0]
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB3_2: // %is_null
not_null:
   %t = load volatile i32, i32* %x, align 4
   ret i32 %t
 }
 
 
 define i8 @imp_null_check_load_i8(i8* %x) {
 ; CHECK-LABEL: imp_null_check_load_i8:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB5_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldrb w0, [x0]
+; CHECK-NEXT:  .Ltmp3:
+; CHECK-NEXT:    ldrb w0, [x0] // on-fault: .LBB5_2
+; CHECK-NEXT:    b .LBB5_1
+; CHECK-NEXT:  .LBB5_1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB5_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i8* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
not_null:
   ret i256 %t
 }
 
 
 
 define i32 @imp_null_check_gep_load(i32* %x) {
 ; CHECK-LABEL: imp_null_check_gep_load:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB7_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldr w0, [x0, #128]
+; CHECK-NEXT:  .Ltmp4:
+; CHECK-NEXT:    ldr w0, [x0, #128] // on-fault: .LBB7_2
+; CHECK-NEXT:    b .LBB7_1
+; CHECK-NEXT:  .LBB7_1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB7_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret i32 42
 
  not_null:
   %x.gep = getelementptr i32, i32* %x, i32 32
   %t = load i32, i32* %x.gep
   ret i32 %t
 }
 
 define i32 @imp_null_check_add_result(i32* %x, i32 %p) {
 ; CHECK-LABEL: imp_null_check_add_result:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB8_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldr w8, [x0]
+; CHECK-NEXT:  .Ltmp5:
+; CHECK-NEXT:    ldr w8, [x0] // on-fault: .LBB8_2
+; CHECK-NEXT:    b .LBB8_1
+; CHECK-NEXT:  .LBB8_1: // %not_null
 ; CHECK-NEXT:    add w0, w8, w1
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB8_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret i32 42
 
  not_null:
   %t = load i32, i32* %x
   %p1 = add i32 %t, %p
   ret i32 %p1
 }
 
 ; Can hoist over a potential faulting instruction as long as we don't
 ; change the conditions under which the instruction faults.
 define i32 @imp_null_check_hoist_over_udiv(i32* %x, i32 %a, i32 %b) {
 ; CHECK-LABEL: imp_null_check_hoist_over_udiv:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB9_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldr w8, [x0]
+; CHECK-NEXT:  .Ltmp6:
+; CHECK-NEXT:    ldr w8, [x0] // on-fault: .LBB9_2
+; CHECK-NEXT:    b .LBB9_1
+; CHECK-NEXT:  .LBB9_1: // %not_null
 ; CHECK-NEXT:    udiv w9, w1, w2
 ; CHECK-NEXT:    add w0, w8, w9
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB9_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret i32 42
 
  not_null:
   %p1 = udiv i32 %a, %b
   %t = load i32, i32* %x
   %res = add i32 %t, %p1
   ret i32 %res
 }
 
 
+; TODO: We should be able to hoist this - we can on x86, why isn't this
+; working for aarch64?  Aliasing?
 define i32 @imp_null_check_hoist_over_unrelated_load(i32* %x, i32* %y, i32* %z) {
 ; CHECK-LABEL: imp_null_check_hoist_over_unrelated_load:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    cbz x0, .LBB10_2
 ; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    ldr w8, [x1]
 ; CHECK-NEXT:    ldr w0, [x0]
 ; CHECK-NEXT:    str w8, [x2]
not_null:
   %t1 = load i32, i32* %x
   store i32 %t0, i32* %z
   ret i32 %t1
 }
 
 define i32 @imp_null_check_gep_load_with_use_dep(i32* %x, i32 %a) {
 ; CHECK-LABEL: imp_null_check_gep_load_with_use_dep:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB11_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldr w8, [x0]
+; CHECK-NEXT:  .Ltmp7:
+; CHECK-NEXT:    ldr w8, [x0] // on-fault: .LBB11_2
+; CHECK-NEXT:    b .LBB11_1
+; CHECK-NEXT:  .LBB11_1: // %not_null
 ; CHECK-NEXT:    add w9, w0, w1
 ; CHECK-NEXT:    add w8, w9, w8
 ; CHECK-NEXT:    add w0, w8, #4 // =4
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB11_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret i32 42
 
  not_null:
   %x.loc = getelementptr i32, i32* %x, i32 1
   %y = ptrtoint i32* %x.loc to i32
   %b = add i32 %a, %y
   %t = load i32, i32* %x
   %z = add i32 %t, %b
   ret i32 %z
 }
 
+;; TODO: We could handle this case as we can lift the fence into the
+;; previous block before the conditional without changing behavior.
 define i32 @imp_null_check_load_fence1(i32* %x) {
 ; CHECK-LABEL: imp_null_check_load_fence1:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    cbz x0, .LBB12_2
 ; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    dmb ishld
 ; CHECK-NEXT:    ldr w0, [x0]
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB12_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
 entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
 is_null:
   ret i32 42
 
 not_null:
   fence acquire
   %t = load i32, i32* %x
   ret i32 %t
 }
 
+;; TODO: We could handle this case as we can lift the fence into the
+;; previous block before the conditional without changing behavior.
 define i32 @imp_null_check_load_fence2(i32* %x) {
 ; CHECK-LABEL: imp_null_check_load_fence2:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    cbz x0, .LBB13_2
 ; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    dmb ish
 ; CHECK-NEXT:    ldr w0, [x0]
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB13_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
 entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
 is_null:
   ret i32 42
 
 not_null:
   fence seq_cst
   %t = load i32, i32* %x
   ret i32 %t
 }
 
+; TODO: We can fold to implicit null here, not sure why this isn't working
 define void @imp_null_check_store(i32* %x) {
 ; CHECK-LABEL: imp_null_check_store:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    cbz x0, .LBB14_2
 ; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    mov w8, #1
 ; CHECK-NEXT:    str w8, [x0]
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB14_2: // %is_null
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret void
 
  not_null:
   store i32 1, i32* %x
   ret void
 }
 
+;; TODO: can be implicit
 define void @imp_null_check_unordered_store(i32* %x) {
 ; CHECK-LABEL: imp_null_check_unordered_store:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    cbz x0, .LBB15_2
 ; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    mov w8, #1
 ; CHECK-NEXT:    str w8, [x0]
 ; CHECK-NEXT:    ret

  not_null:
   store atomic i32 1, i32* %x unordered, align 4
   ret void
 }
 
 define i32 @imp_null_check_neg_gep_load(i32* %x) {
 ; CHECK-LABEL: imp_null_check_neg_gep_load:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cbz x0, .LBB16_2
-; CHECK-NEXT:  // %bb.1: // %not_null
-; CHECK-NEXT:    ldur w0, [x0, #-128]
+; CHECK-NEXT:  .Ltmp8:
+; CHECK-NEXT:    ldur w0, [x0, #-128] // on-fault: .LBB16_2
+; CHECK-NEXT:    b .LBB16_1
+; CHECK-NEXT:  .LBB16_1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB16_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0