[Attributor] Deduction based on path exploration

[uenoku]

Summary

This patch introduces the propagation of known information based on path exploration.
For example,

int u(int c, int *p){
  if(c) {
     return *p;
  } else {
     return *p + 1;
  }
}

An argument p is dereferenced whatever c's value is.

For an instruction CtxI, we accumulate branch instructions in the must-be-executed-context of CtxI and then, we take the conjunction of the successors' known state.

[uenoku]

Fix typo.

[jdoerfert]

I only reviewed the MustExecute parts as the Attributor part probably needs a rebase. I think this is needed and I'll be more responsive this time.

llvm/include/llvm/Analysis/MustExecute.h

395

Newline before and comment describing this struct and its use briefly.

411

I would not call it Multiplicative/Additive identity, nor Top/Bottom. I would stick to the Attributor spellings of "best/worst state". The others are overloaded too often (IMHO).

429

If you keep pointer values, this is shorter: if (AState *State = MemoizeResult.lookup(PPBegin)) return *State;

452

I think I would prefer that we first explore range completely and only collect branch instructions. If range was not enough to cause stopExplore to trigger, we start exploring branches. The reason is that range information is "better" than child information as it holds without the need to explore a second path. We should also have a parameter to limit the search.

454

Is it wise to store the address of the local object AS here? Maybe we need the map to own the state.

[uenoku]

Rebased. I simply use AbstractState to accumulate the known information.

[baziotis]

That's very interesting, I hope I'll have time to come back with a better review.

llvm/lib/Transforms/IPO/Attributor.cpp

510–511

This UsesInChild I think is not used anywhere. Probably put there since something has to be passed to the function. Can we do something to avoid inserting to it and deleting in every loop? (If I haven't missed something)

[jdoerfert]

Thanks for resurrecting this patch. This gives us really interesting opportunities as we can already see with the nonnull and deref changes :)
I will need to look over everything, maybe next week (I'm on travel again). A first comment inlined though.

llvm/include/llvm/Analysis/MustExecute.h

395

My first impression is that we might want to shift the burden of collecting instructions to the user and just do the iterating here. The benefit is that we could use the return of Pred to indicate that we want to stop. I think we want to have a way to stop the search of the context if we don't expect to find better results anymore.

[uenoku]

Address comments

[uenoku]

llvm/include/llvm/Analysis/MustExecute.h

395

Addressed.

llvm/lib/Transforms/IPO/Attributor.cpp

510–511

Yes, that's right.

[baziotis]

llvm/lib/Transforms/IPO/Attributor.cpp

510–511

Thanks for addressing this. I think it still isn't used though right? i.e. With your update, it seems you can just remove UsesInChild declaration completely.

[uenoku]

llvm/lib/Transforms/IPO/Attributor.cpp

510–511

Oh, I forgot to remove it ;)

[jdoerfert]

llvm/include/llvm/Transforms/IPO/Attributor.h

1342 ↗

(On Diff #246645)

We need Documentation for this.

llvm/lib/Transforms/IPO/Attributor.cpp

474

Can you explain the NOTE further and why we cannot make it "right"?

---

Do you really want to delete UsesInChild or do you want to use it in the followUsesInContext call such that Uses is not modifed here.

[baziotis]

llvm/lib/Transforms/IPO/Attributor.cpp

474

Do you really want to delete UsesInChild or do you want to use it in the followUsesInContext call such that Uses is not modifed here.

Hmm, that was my initial assumption as well:

Probably put there since something has to be passed to the function

but I thought I was wrong.

[uenoku]

Simplify implementation.

[uenoku]

llvm/lib/Transforms/IPO/Attributor.cpp

474

Can you explain the NOTE further and why we cannot make it "right"?

I didn't have a brain;) I have implemented in a simpler way so NOTE is removed.

Do you really want to delete UsesInChild or do you want to use it in the followUsesInContext call such that Uses is not modifed here.

I want to modify Uses in followUsesInContext so I'm fine with removing UsesInChild.

[jdoerfert]

I have not convinced myself that this covers the recursive case properly, maybe that was never the intend though. Can we have a test like this:

void foo(int a, int b, int c, int *ptr) {
  if (a) {
    if (b)
      *ptr = 1;
    else
      *ptr = 2;
  } else {
    if (c)
      *ptr = 3;
    else
      *ptr = 4;
  }
}

and maybe also

void foo(int a, int b, int c, int *ptr) {
  if (a) {
    if (b)
      *ptr = 1;
    else
      *ptr = 2;
  } else {
    if (c)
      *ptr = 3;
    else
      foo(1, 1, 1, ptr);
  }
}

llvm/lib/Transforms/IPO/Attributor.cpp

492

I don't get the comment. What are the two indices of ChildState and why is the second ChildState "out-of-sync"?

[uenoku]

Fix comment and add test.

[uenoku]

I have not convinced myself that this covers the recursive case properly, maybe that was never the intend though. Can we have a test like this:

Yes, recursive branches are not handled in this patch to simplify. I'll add on a later patch.

llvm/lib/Transforms/IPO/Attributor.cpp

492

It was broken. Sorry for the confusion.

[jdoerfert]

LGTM. We need a recursive lookup that tracks paths up to a given depth next.

llvm/include/llvm/Analysis/MustExecute.h

struct MustBeExecutedContextExplorer {
   ///
   /// \param It              The iterator that is used to traverse the must be
   ///                        executed context.
   /// \param PP              The program point for which the next instruction
   ///                        that is guaranteed to execute is determined.
   const Instruction *
   getMustBeExecutedNextInstruction(MustBeExecutedIterator &It,
                                    const Instruction *PP);
+  struct KnownAbstractState {
+    virtual ~KnownAbstractState () = default;
+
+    /// Conjunction for state.
+    KnownAbstractState &operator&=(const KnownAbstractState &);
+
+    /// Disjunction for state.
+    KnownAbstractState &operator|=(const KnownAbstractState &);
+
+    /// Return true if there is no need to explore more path.
+    virtual bool stopExplore() const;
+
+    /// Multiplicative identity.
+    static KnownAbstractState Top();
+
+    /// Additive identity.
+    static KnownAbstractState Bottom();
+  };
+
+  /// Explore the pathes which must be executed after \p PP and collect states.
+  ///
+  /// \param Pred            The function that takes Instruction and return
+  ///                        KnownAbstractState.
+  /// \param PP              The program point for which the next instruction
+  ///                        that is guaranteed to execute is determined.
+  template <typename AState>
+  AState checkPredicateAfterInstruction(
+      std::function<AState(const Instruction *I)> Pred, const Instruction *PP,
+      DenseMap<const Instruction *, AState*> &MemoizeResult) {
+
+    // For each program point, we regard *begin(PP) as ID for the context.
+    const Instruction *PPBegin = *begin(PP);
+
+    if (MemoizeResult.count(PPBegin))
+      return *MemoizeResult.lookup(PPBegin);
+
+    AState AS = AState::Bottom();
+
+    // Traverse all must-be-excuted-context and take disjunction of predicate.
+    // If we reach to conditional terminator, call this function recursively and
+    // take conjunction of children's state.
+    //
+    // (I_1 | I_2 | I_3 .. I_n) | (C_1 & C_2 & C_3 .. C_n)
+
+    for (const Instruction *I : range(PP)) {
+      AS |= Pred(I);
+      if (AS.stopExplore())
+        return AS;
+
+      if (I->isTerminator() && I->getNumSuccessors() > 1) {
+        MemoizeResult[PPBegin] = &AS;
+        AState ChildrenAA = AState::Top();
+        for (const BasicBlock *BB : successors(I))
+          ChildrenAA &= checkPredicateAfterInstruction<AState>(
+              Pred, &BB->front(), MemoizeResult);
+        AS |= ChildrenAA;
+      }
+    }
+
+    MemoizeResult[PPBegin] = &AS;
+    return AS;
+  }
+
+  template <typename AAType>
+  AAType checkPredicateAfterInstruction(
+      std::function<AAType(const Instruction *I)> Pred, const Instruction *PP) {
+    DenseMap<const Instruction *, AAType*> MemoizeResult;
+    return checkPredicateAfterInstruction<AAType>(Pred, PP, MemoizeResult);
+  }
 
-  /// Return true if there is a instruction that \p Pred holds in the context
-  /// around \p PP.
-  bool
-  checkPredicateAfterInstruction(std::function<bool(const Instruction *)> Pred,
-                                 const Instruction *PP,
-                                 bool ContinueExplorationWhenPredHolds = false);
 
   /// Parameter that limit the performed exploration. See the constructor for
   /// their meaning.
   ///{
   const bool ExploreInterBlock;
   ///}
 
 private:

llvm/lib/Analysis/MustExecute.cpp

LLVM_DEBUG(
         dbgs() << "\tUnconditional terminator, continue with successor\n");
     return &PP->getSuccessor(0)->front();
   }
 
   LLVM_DEBUG(dbgs() << "\tNo join point found\n");
   return nullptr;
 }
 
-bool MustBeExecutedContextExplorer::checkPredicateAfterInstruction(
-    std::function<bool(const Instruction *)> Pred, const Instruction *PP,
-    bool ContinueExplorationWhenPredHolds) {
-  bool PredHolds = false;
-  for (const Instruction *I : range(PP)) {
-    LLVM_DEBUG(dbgs() << "[MustBeExecuted][checkPred] PP: " << *PP
-                      << " I:" << *I << "\n");
-
-    PredHolds |= Pred(I);
-    if (PredHolds && !ContinueExplorationWhenPredHolds)
-      return true;
-  }
-  return PredHolds;
-}
 
 MustBeExecutedIterator::MustBeExecutedIterator(
     MustBeExecutedContextExplorer &Explorer, const Instruction *I)
     : Explorer(Explorer), CurInst(I) {
   reset(I);
 }
 
 void MustBeExecutedIterator::reset(const Instruction *I) {

llvm/lib/Transforms/IPO/Attributor.cpp

if (!Ptr)
     return nullptr;
 
   return GetPointerBaseWithConstantOffset(Ptr, BytesOffset, DL,
                                           /*AllowNonInbounds*/ false);
 }
 
 /// -----------------------NoUnwind Function Attribute--------------------------
 
 struct AANoUnwindFunction : AANoUnwind, BooleanState {
 
   AANoUnwindFunction(Function &F, InformationCache &InfoCache)
       : AANoUnwind(F, InfoCache) {}
 
   /// See AbstractAttribute::getState()
   /// {
   AbstractState &getState() override { return *this; }
   const AbstractState &getState() const override { return *this; }
   /// }
 
   /// See AbstractAttribute::getManifestPosition().
   ManifestPosition getManifestPosition() const override { return MP_FUNCTION; }
 
   const std::string getAsStr() const override {
     return getAssumed() ? "nounwind" : "may-unwind";
   }
 
   /// See AbstractAttribute::updateImpl(...).
   ChangeStatus updateImpl(Attributor &A) override;
 
   /// See AANoUnwind::isAssumedNoUnwind().
   bool isAssumedNoUnwind() const override { return getAssumed(); }
 
   /// See AANoUnwind::isKnownNoUnwind().
   bool isKnownNoUnwind() const override { return getKnown(); }
 };
 
 ChangeStatus AANoUnwindFunction::updateImpl(Attributor &A) {
   Function &F = getAnchorScope();
 
   // The map from instruction opcodes to those instructions in the function.
   auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
   auto Opcodes = {
       (unsigned)Instruction::Invoke,      (unsigned)Instruction::CallBr,
       (unsigned)Instruction::Call,        (unsigned)Instruction::CleanupRet,
       (unsigned)Instruction::CatchSwitch, (unsigned)Instruction::Resume};
 
   for (unsigned Opcode : Opcodes) {
     for (Instruction *I : OpcodeInstMap[Opcode]) {
       if (!I->mayThrow())
         continue;
 
       auto *NoUnwindAA = A.getAAFor<AANoUnwind>(*this, *I);
 
       if (!NoUnwindAA || !NoUnwindAA->isAssumedNoUnwind()) {
struct AANonNullCallSiteArgument : AANonNullImpl {
   };
 
   // Return argument index of associated value.
   int getArgNo() const { return ArgNo; }
 
 private:
   unsigned ArgNo;
 };
+
+struct ASBool : MustBeExecutedContextExplorer::KnownAbstractState {
+  bool State;
+  ASBool(bool State) : State(State) {}
+  bool stopExplore() const override { return State; }
+  ASBool &operator&=(const ASBool R) {
+    State &= R.State;
+    return *this;
+  }
+  ASBool &operator|=(const ASBool R) {
+    State |= R.State;
+    return *this;
+  }
+  static ASBool Bottom() { return ASBool(false); }
+  static ASBool Top() { return ASBool(true); }
+};
+
 ChangeStatus AANonNullArgument::updateImpl(Attributor &A) {
   Function &F = getAnchorScope();
   Argument &Arg = cast<Argument>(getAnchoredValue());
 
   unsigned ArgNo = Arg.getArgNo();
 
   MustBeExecutedContextExplorer &Explorer = InfoCache.getContextExplorer();
 
-  auto Pred = [&](const Instruction *I) -> bool {
+  std::function<ASBool(const Instruction *)> Pred =
+      [&](const Instruction *I) -> ASBool {
     // See callsite abstract attribute.
     if (ImmutableCallSite ICS = ImmutableCallSite(I))
       for (unsigned int i = 0; i < ICS.getNumArgOperands(); i++)
         if (ICS.getArgOperand(i) == &getAnchoredValue())
           if (auto *NonNullAA = A.getAAFor<AANonNull>(*this, *I, i))
             // FIXME: Use assumption.
-            return NonNullAA->isKnownNonNull();
+            return ASBool(NonNullAA->isKnownNonNull());
 
     // See memory instruction with constant offset.
     // Currently only inbounds GEPs are tracked.
     // TODO: Trace non-inbounds GEPs.
     int64_t Offset = 0;
     if (const Value *Base = getBasePointerOfPointerOperand(
             I, Offset, getAnchorScope().getParent()->getDataLayout()))
-      return Base == &getAnchoredValue();
+      return ASBool(Base == &getAnchoredValue());
 
-    return false;
+    return ASBool::Bottom();
   };
 
-  if (Explorer.checkPredicateAfterInstruction(
-          Pred, &getAnchorScope().getEntryBlock().front())) {
+  if (Explorer
+          .checkPredicateAfterInstruction<ASBool>(
+              Pred, &getAnchorScope().getEntryBlock().front())
+          .State) {
     indicateOptimisticFixpoint();
     return ChangeStatus::CHANGED;
   }
 
   // Callback function
   std::function<bool(CallSite)> CallSiteCheck = [&](CallSite CS) {
     assert(CS && "Sanity check: Call site was not initialized properly!");
 
struct AADereferenceableArgument : AADereferenceableImpl {
 
   /// See AbstractAttribute::getManifestPosition().
   ManifestPosition getManifestPosition() const override { return MP_ARGUMENT; }
 
   /// See AbstractAttribute::updateImpl(...).
   ChangeStatus updateImpl(Attributor &A) override;
 };
 
+struct ASDeref : MustBeExecutedContextExplorer::KnownAbstractState {
+  bool NonNull;
+  uint32_t DerefBytes;
+  ASDeref(bool NonNull, uint32_t DerefBytes)
+      : NonNull(NonNull), DerefBytes(DerefBytes) {}
+
+  bool stopExplore() const override { return false; }
+
+  ASDeref &operator&=(const ASDeref &R) {
+    NonNull &= R.NonNull;
+    DerefBytes = std::min(DerefBytes, R.DerefBytes);
+    return *this;
+  }
+  ASDeref &operator|=(const ASDeref &R) {
+    NonNull |= R.NonNull;
+    DerefBytes = std::max(DerefBytes, R.DerefBytes);
+    return *this;
+  }
+  static ASDeref Bottom() { return ASDeref(false, 0); }
+  static ASDeref Top() { return ASDeref(true, 1e9); }
+};
+
 ChangeStatus AADereferenceableArgument::updateImpl(Attributor &A) {
   Function &F = getAnchorScope();
   const DataLayout &DL = F.getParent()->getDataLayout();
 
   Argument &Arg = cast<Argument>(getAnchoredValue());
 
   auto BeforeState = static_cast<DerefState>(*this);
 
   unsigned ArgNo = Arg.getArgNo();
 
   syncNonNull(A.getAAFor<AANonNull>(*this, F, ArgNo));
 
   bool IsNonNull = isAssumedNonNull();
   bool IsGlobal = isAssumedGlobal();
 
   MustBeExecutedContextExplorer &Explorer = InfoCache.getContextExplorer();
 
-  auto Pred = [&](const Instruction *I) {
+  std::function<ASDeref(const Instruction *I)> Pred =
+      [&](const Instruction *I) -> ASDeref {
     // See callsite abstract attribute.
+    ASDeref Deref = ASDeref::Bottom();
     if (ImmutableCallSite ICS = ImmutableCallSite(I))
       for (unsigned int i = 0; i < ICS.getNumArgOperands(); i++)
         if (ICS.getArgOperand(i) == &getAnchoredValue())
-          if (auto *DerefAA = A.getAAFor<AADereferenceable>(*this, *I, i)) {
-            // FIXME: Use assumption.
-            auto DerefBytes = DerefAA->getKnownDereferenceableBytes();
-            takeKnownDerefBytesMaximum(DerefBytes);
-            return true;
-          }
+          if (auto *DerefAA = A.getAAFor<AADereferenceable>(*this, *I, i))
+            Deref |= ASDeref(DerefAA->isKnownNonNull(),
+                             DerefAA->getKnownDereferenceableBytes());
 
     // See memory instruction with constant offset.
     // Currently only inbounds GEPs are tracked.
     // TODO: Trace non-inbounds GEPs.
     int64_t Offset = 0;
     if (const Value *Base = getBasePointerOfPointerOperand(I, Offset, DL))
       if (Base == &getAnchoredValue()) {
         uint32_t DerefBytes =
             Offset +
             DL.getTypeStoreSize(Base->getType()->getPointerElementType());
-        addKnownNonnull();
-        takeKnownDerefBytesMaximum(DerefBytes);
-        return true;
+        Deref |= ASDeref(true, DerefBytes);
       }
-    return false;
+    return Deref;
   };
 
-  Explorer.checkPredicateAfterInstruction(
-      Pred, &getAnchorScope().getEntryBlock().front(),
-      /* ContinueExplorationWhenPredHolds */ true);
+  ASDeref Res = Explorer.checkPredicateAfterInstruction<ASDeref>(
+      Pred, &getAnchorScope().getEntryBlock().front());
+
+  if (Res.NonNull)
+    addKnownNonnull();
+  takeKnownDerefBytesMaximum(Res.DerefBytes);
 
   // Callback function
   std::function<bool(CallSite)> CallSiteCheck = [&](CallSite CS) -> bool {
     assert(CS && "Sanity check: Call site was not initialized properly!");
 
     // Check that DereferenceableAA is AADereferenceableCallSiteArgument.
     if (auto *DereferenceableAA =
             A.getAAFor<AADereferenceable>(*this, *CS.getInstruction(), ArgNo)) {

llvm/test/Transforms/FunctionAttrs/dereferenceable.ll

 ; TEST 4
 ; Better than known in IR.
 
 define dereferenceable(4) i32* @test4(i32* dereferenceable(8)) local_unnamed_addr {
 ; ATTRIBUTOR: define nonnull dereferenceable(8) i32* @test4(i32* nonnull returned dereferenceable(8))
   ret i32* %0
 }
 
+declare void @fun0() #0
+declare void @fun1(i8*) #0
+declare void @fun2(i8*, i8*) #0
+declare void @fun3(i8*, i8*, i8*) #0
+; TEST 5 simple path test
+; if(..)
+;   fun2(dereferenceable(8) %a, dereferenceable(8) %b)
+; else
+;   fun2(dereferenceable(4) %a, %b)
+; We can say that %a is dereferenceable(4) but %b is not.
+define void @f5(i8* %a, i8 * %b, i8 %c) {
+; ATTRIBUTOR: define void @f5(i8* nonnull dereferenceable(4) %a, i8* %b, i8 %c)
+  %cmp = icmp eq i8 %c, 0
+  br i1 %cmp, label %if.then, label %if.else
+if.then:
+  tail call void @fun2(i8* dereferenceable(8) %a, i8* dereferenceable(8) %b)
+  ret void
+if.else:
+  tail call void @fun2(i8* dereferenceable(4) %a, i8* %b)
+  ret void
+}
+; TEST 6 explore child BB test
+; if(..)
+;    ... (willreturn & nounwind)
+; else
+;    ... (willreturn & nounwind)
+; fun1(dereferenceable(8) %a)
+; We can say that %a is dereferenceable(8)
+define void @f6(i8* %a, i8 %c) {
+; ATTRIBUTOR: define void @f6(i8* nonnull dereferenceable(8) %a, i8 %c)
+  %cmp = icmp eq i8 %c, 0
+  br i1 %cmp, label %if.then, label %if.else
+if.then:
+  tail call void @fun0()
+  br label %cont
+if.else:
+  tail call void @fun0()
+  br label %cont
+cont:
+  tail call void @fun1(i8* dereferenceable(8) %a)
+  ret void
+}
+; TEST 7 More complex test
+; {
+; fun1(dereferenceable(4) %a)
+; if(..)
+;    ... (willreturn & nounwind)
+;    fun1(dereferenceable(12) %a)
+; else
+;    ... (willreturn & nounwind)
+;    fun1(dereferenceable(16) %a)
+; fun1(dereferenceable(8) %a)
+; }
+; %a is dereferenceable(12)
+
+define void @f7(i8* %a, i8* %b, i8 %c) {
+; ATTRIBUTOR: define void @f7(i8* nonnull dereferenceable(12) %a, i8* %b, i8 %c)
+  %cmp = icmp eq i8 %c, 0
+  tail call void @fun1(i8* dereferenceable(4) %a)
+  br i1 %cmp, label %cont.then, label %cont.else
+cont.then:
+  tail call void @fun1(i8* dereferenceable(12) %a)
+  br label %cont2
+cont.else:
+  tail call void @fun1(i8* dereferenceable(16) %a)
+  br label %cont2
+cont2:
+  tail call void @fun1(i8* dereferenceable(8) %a)
+  ret void
+}
+define void @f8(i8* %a) {
+  tail call void @fun1(i8* dereferenceable(4) %a)
+  ret void
+}
+
+attributes #0 = { nounwind willreturn}

llvm/test/Transforms/FunctionAttrs/nonnull.ll

 declare void @fun3(i8*, i8*, i8*) #1
 ; TEST 16 simple path test
 ; if(..)
 ;   fun2(nonnull %a, nonnull %b)
 ; else
 ;   fun2(nonnull %a, %b)
 ; We can say that %a is nonnull but %b is not.
 define void @f16(i8* %a, i8 * %b, i8 %c) {
-; FIXME: missing nonnull on %a
-; ATTRIBUTOR: define void @f16(i8* %a, i8* %b, i8 %c)
+; ATTRIBUTOR: define void @f16(i8* nonnull %a, i8* %b, i8 %c)
   %cmp = icmp eq i8 %c, 0
   br i1 %cmp, label %if.then, label %if.else
 if.then:
   tail call void @fun2(i8* nonnull %a, i8* nonnull %b)
   ret void
 if.else:
   tail call void @fun2(i8* nonnull %a, i8* %b)
   ret void
 }
 ; TEST 17 explore child BB test
 ; if(..)
 ;    ... (willreturn & nounwind)
 ; else
 ;    ... (willreturn & nounwind)
 ; fun1(nonnull %a)
 ; We can say that %a is nonnull
 define void @f17(i8* %a, i8 %c) {
-; FIXME: missing nonnull on %a
-; ATTRIBUTOR: define void @f17(i8* %a, i8 %c)
+; ATTRIBUTOR: define void @f17(i8* nonnull %a, i8 %c)
   %cmp = icmp eq i8 %c, 0
   br i1 %cmp, label %if.then, label %if.else
 if.then:
   tail call void @fun0()
   br label %cont
 if.else:
   tail call void @fun0()
   br label %cont
 cont:
   tail call void @fun1(i8* nonnull %a)
   ret void
 }
 ; TEST 18 More complex test
 ; if(..)
 ;    ... (willreturn & nounwind)
 ; else
 ;    ... (willreturn & nounwind)
 ; if(..)
 ;    ... (willreturn & nounwind)
 ; else
 ;    ... (willreturn & nounwind)
 ; fun1(nonnull %a)
 
 define void @f18(i8* %a, i8* %b, i8 %c) {
-; FIXME: missing nonnull on %a
-; ATTRIBUTOR: define void @f18(i8* %a, i8* %b, i8 %c)
+; ATTRIBUTOR: define void @f18(i8* nonnull %a, i8* %b, i8 %c)
   %cmp1 = icmp eq i8 %c, 0
   br i1 %cmp1, label %if.then, label %if.else
 if.then:
   tail call void @fun0()
   br label %cont
 if.else:
   tail call void @fun0()
   br label %cont

 cont2:
   tail call void @fun1(i8* nonnull %a)
   ret void
 }
 
 ; TEST 19: Loop
 
 define void @f19(i8* %a, i8* %b, i8 %c) {
-; FIXME: missing nonnull on %b
-; ATTRIBUTOR: define void @f19(i8* %a, i8* %b, i8 %c)
+; ATTRIBUTOR: define void @f19(i8* %a, i8* nonnull %b, i8 %c)
   br label %loop.header
 loop.header:
   %cmp2 = icmp eq i8 %c, 0
   br i1 %cmp2, label %loop.body, label %loop.exit
 loop.body:
   tail call void @fun1(i8* nonnull %b)
   tail call void @fun1(i8* nonnull %a)
   br label %loop.header