[IR] Adds mustprogress as a LLVM IR attribute

[atmnpatel]

Summary

This adds the LLVM IR attribute mustprogress as defined in LangRef through D86233. This attribute will be applied to functions with in languages like C++ where forward progress is guaranteed. Functions without this attribute are not required to make progress.

[atmnpatel]

Removed extra file.

[jdoerfert]

This was quick, we need to write an RFC now ;). For this we should probably write the lang ref part too.
We should also mention somewhere this is the first step to properly fix PR965 and make the llvm.sideeffect obsolete (for this use case).

Adding some people that should be aware.

[RalfJung]

I am going solely off the description here as I am not familiar with LLVM internals. Looking forward to the LangRef update. :)

Great to see some progress (heh ;) is made on this, thanks Johannes! One thing made me curious though: if I understand correctly "noprogress" is a somewhat strange attribute in that *not* having this attribute promises that the function *will* "make progress"? That's the opposite from other attributes like "nounwind". Is it not possible to instead have a "progress" or "nodiverge" attribute which promises that the function *will* make progress / will not diverge (in the sense of running infinitely without any side-effect)? I imagine the attribute could be added by default when loading old bitcode to preserve semantics (but dropping the attribute is safe so this would only be crucial for performance, not for correctness).

Also, in terms of semantics (but this might be more appropriate for the LangRef PR), is it legal for a "progress" and "noprogress" function to mutually recursively call each other infinitely? As in, does not having "noprogress" promise that every call to this function will return or have infinitely many side-effects, or does not having "noprogress" just promise that *this function* and all the no-"noprogress" functions it calls will not diverge? (You can see here why I think the attribute should be inverted in its meaning. ;)

[jdoerfert]

In D85393#2199024, @RalfJung wrote:

I am going solely off the description here as I am not familiar with LLVM internals. Looking forward to the LangRef update. :)

Great to see some progress (heh ;) is made on this, thanks Johannes! One thing made me curious though: if I understand correctly "noprogress" is a somewhat strange attribute in that *not* having this attribute promises that the function *will* "make progress"? That's the opposite from other attributes like "nounwind". Is it not possible to instead have a "progress" or "nodiverge" attribute which promises that the function *will* make progress / will not diverge (in the sense of running infinitely without any side-effect)? I imagine the attribute could be added by default when loading old bitcode to preserve semantics (but dropping the attribute is safe so this would only be crucial for performance, not for correctness).

Right, the "direction" of the attribute is what we need to discuss now. This was a first try, renaming it now is simple if we decide to do so. I suggested to start with this version as it shouldn't cause regressions for C/C++, though, I acknowledge the opposite attribute might at first be the "safer"/"saner" choice. Either should do the trick after a while given that all optimizations need to preserve/handle attribute already, you cannot just drop them and so noprogress should be sufficient.

Also, in terms of semantics (but this might be more appropriate for the LangRef PR), is it legal for a "progress" and "noprogress" function to mutually recursively call each other infinitely? As in, does not having "noprogress" promise that every call to this function will return or have infinitely many side-effects, or does not having "noprogress" just promise that *this function* and all the no-"noprogress" functions it calls will not diverge? (You can see here why I think the attribute should be inverted in its meaning. ;)

Hm... I will need to think about this. Given that we want to mix C/C++ with Rust on IR-level, right?

[RalfJung]

Either should do the trick after a while given that all optimizations need to preserve/handle attribute already, you cannot just drop them and so noprogress should be sufficient.

Ah, I thought so far attributes were always safe to drop. If that is not the case, what remains is the possible confusion on our side when having to talk about "noprogress" and no-"noprogress" functions, and the asymmetry with most (but seemingly not all) existing attributes.

Hm... I will need to think about this. Given that we want to mix C/C++ with Rust on IR-level, right?

That would be an example, yes -- cross-language LTO is used e.g. by Firefox. From the normative perspective, neither the C++ spec nor the Rust spec say what happens when a C++ and Rust function form a mutually infinite recursion. My personal preference is to allow such behavior, i.e., infinite recursion is only UB if *all* involved functions are no-"noprogress". But that is mostly my personal bias against making things UB, I cannot say how this affects analyses/optimizations.

(I am assuming here that stack overflows are already handled e.g. by a guard page, which means that in pure Rust code, infinite recursion is well-defined, and this must be handled appropriately by optimizations.)

[jdoerfert]

In D85393#2200054, @RalfJung wrote:

Either should do the trick after a while given that all optimizations need to preserve/handle attribute already, you cannot just drop them and so noprogress should be sufficient.

Ah, I thought so far attributes were always safe to drop. If that is not the case, what remains is the possible confusion on our side when having to talk about "noprogress" and no-"noprogress" functions, and the asymmetry with most (but seemingly not all) existing attributes.

Just FYI, convergent is one that cannot be dropped. Also naked, nobuiltin, noduplicate, noimplicitfloat, null_pointer_is_valid, ... and some of them have to survive inlining already as well.

[aqjune]

Do we need a well-formedness check at Verifier.cpp stating that noprogress and willreturn cannot exist together?

[jdoerfert]

In D85393#2205746, @aqjune wrote:

Do we need a well-formedness check at Verifier.cpp stating that noprogress and willreturn cannot exist together?

Let's first create the Lang Ref patch before we dive into details here. Generally, we will need verifier checks, yes. FWIW, noprogress is unrelated to willreturn and noreturn though.

[aqjune]

In D85393#2205762, @jdoerfert wrote:

Let's first create the Lang Ref patch before we dive into details here. Generally, we will need verifier checks, yes. FWIW, noprogress is unrelated to willreturn and noreturn though.

IIUC, a function makes progress if the function has willreturn - wouldn't it make noprogress give no information?
It isn't contradiction, but about whether noprogress brings useful information or not, though.

[RalfJung]

It isn't contradiction, but about whether noprogress brings useful information or not, though.

The presence of "noprogress" doesn't bring much information, but its absence does.

Even if we keep the polarity, maybe it would be better to call it "maybenoprogress" or "maybeprogress"? After all, "noprogress" sounds like it asserts that the function will not make progress, when really the flag that that the function *is permitted to make no progress* (unlike functions without this attribute, which have to make progress).

[jdoerfert]

In D85393#2206044, @RalfJung wrote:

It isn't contradiction, but about whether noprogress brings useful information or not, though.

The presence of "noprogress" doesn't bring much information, but its absence does.

It depends on how you look at it:
From the perspective of optimizations we can do, sure absence is the one that enables more.
From the perspective of source behavior, it just distinguishes between two alternatives, so both option w/ and w/o flag have the same information value.

Even if we keep the polarity, maybe it would be better to call it "maybenoprogress" or "maybeprogress"? After all, "noprogress" sounds like it asserts that the function will not make progress, when really the flag that that the function *is permitted to make no progress* (unlike functions without this attribute, which have to make progress).

That is fair. maybe... or mayprogress or mayloop or mayspin? Naming things is hard ...

In D85393#2205925, @aqjune wrote:

In D85393#2205762, @jdoerfert wrote:

Let's first create the Lang Ref patch before we dive into details here. Generally, we will need verifier checks, yes. FWIW, noprogress is unrelated to willreturn and noreturn though.

IIUC, a function makes progress if the function has willreturn - wouldn't it make noprogress give no information?
It isn't contradiction, but about whether noprogress brings useful information or not, though.

So this is a may/must difference and these attributes also do not perfectly match. However:
willreturn implies must-make-progress (as it cannot loop), which then allows you to remove this attribue (maybe-no-progress) as it actually will make progress.
Even when willreturn is not applicable you can have must-make-progress. It basically is the attribute we implicitly assume right now. Though I don't have a reason handy why you would derive the must-make-progress and remove the may-... version. If the user/input gives you both, that is a different story though.

[RalfJung]

It depends on how you look at it:
From the perspective of optimizations we can do, sure absence is the one that enables more.
From the perspective of source behavior, it just distinguishes between two alternatives, so both option w/ and w/o flag have the same information value.

I don't entirely understand what you mean. From the perspective of source behavior, *removing* the attribute adds UB, i.e., it turns some previously non-UB executions into UB executions. That's exactly the opposite of attributes like "noreturn" or "nosync" or "nounwind" that all add UB when being *added*.

[jdoerfert]

In D85393#2241230, @RalfJung wrote:

It depends on how you look at it:
From the perspective of optimizations we can do, sure absence is the one that enables more.
From the perspective of source behavior, it just distinguishes between two alternatives, so both option w/ and w/o flag have the same information value.

I don't entirely understand what you mean. From the perspective of source behavior, *removing* the attribute adds UB, i.e., it turns some previously non-UB executions into UB executions. That's exactly the opposite of attributes like "noreturn" or "nosync" or "nounwind" that all add UB when being *added*.

I don't look at it as added or removed, for me it is a boolean flag, with a default that requires progress. It is more like nobuiltin, noduplicate, nomerge, returns_twice, .... The difference is that I don't see this being deduced. It is just a description of the input semantics, not a property we derive from the code (to describe the code). Unclear if the view makes a difference, let me know what u think :)

[RalfJung]

I don't look at it as added or removed, for me it is a boolean flag, with a default that requires progress. It is more like nobuiltin, noduplicate, nomerge, returns_twice, .... The difference is that I don't see this being deduced. It is just a description of the input semantics, not a property we derive from the code (to describe the code). Unclear if the view makes a difference, let me know what u think :)

I don't think of any of these attributes as being deduced. ;) I don't care where the attributes come from, the important bit is how they affect the language semantics, the language here being LLVM IR of course. (It's almost as if I'm a programming language researcher. ;)

nounwind and friends affect the language semantics by saying things like: if this function unwinds, that's UB. (In the follows that if you can deduce that a function doesn't unwind, it is okay to add the attribute. But as with all optimizations and analysis, they are not the ground truth, they have to be justified against the language semantics.)
The *absence* of noprogress affects the language semantics by saying: if this function runs into a side-effect-free infinite loop, that's UB. (And something about infinite recursion, but that's a bit more tricky because multiple functions can be involved.) Adding noprogress disables that UB. (This makes me prefer "allownoprogress" as attribute name, if you want to keep its polarity.)

That's how this attribute is "the other way around" compared to the normal ones. Unfortunately I don't know what nobuiltin, nomerge etc mean, so I cannot say how they affect language semantics. I don't know what you mean by "input semantics".

noprogress could certainly be deduced like the others, it's just backwards again -- we have to deduce its absence. For a function that has noprogress, if the compiler can prove that there is no infinite loop, it may remove the attribute. (I am not sure if that's a useful thing to do, I am just saying it is possible.)

[jdoerfert]

I'm fine with the other name and I removed my long answer because I don't think we were going anywhere.

[atmnpatel]

Renamed the IR attribute to mayprogress from noprogress because it makes it clearer that the forward progress guarantee is relaxed.

[atmnpatel]

This comment was removed by atmnpatel.

[RalfJung]

Changed name from noprogress to mayprogress for clarity.

Shouldn't it be "maynotprogress" (or "maybenoprogress" or so)? *Every* function *may* progress, but only those with this marker are also allowed to not "make progress".

[atmnpatel]

Renamed to maynotprogress.

[atmnpatel]

Change direction.

[atmnpatel]

Rebase.

[nikic]

llvm/lib/AsmParser/LLParser.cpp

1374

It looks like the simple attributes are sorted alphabetically above. Can you please insert this at the alphabetical position?

1761

Just add this to the preceding case list?

1878

Same here, add this to the existing "invalid use of function-only attribute" case list?

[atmnpatel]

Addressed inline comments.

[atmnpatel]

Updated definition of hasMustProgress to also include willreturn as per the LangRef definition.

[nikic]

llvm/include/llvm/IR/Attributes.td

238

This comment is out of date.

llvm/include/llvm/IR/Function.h

626

The doc comment here is out of date. Might also want to rename to mustProgress() or isMustProgress() as it is not a pure check of the attribute.

llvm/test/Assembler/mustprogress-parse-error-2.ll

5

Possibly this test should be something like define i32* @test_mustprogress(i8 %a) mustprogress 8 {? Otherwise it runs in the "function-only" error first.

llvm/test/Assembler/mustprogress-parse-error-3.ll

4 ↗

(On Diff #294454)

How does this differ from llvm/test/Assembler/mustprogress-parse-error-1.ll?

[atmnpatel]

Fixes.

[nikic]

LGTM

llvm/include/llvm/IR/Function.h

625

Nit: Missing period at end of sentence.

[atmnpatel]

rebase.

[atmnpatel]

addressed nit.

[atmnpatel]

attempt 2, ignore

[Meinersbur]

Should there be some documentation about the new attribute?

[atmnpatel]

In D85393#2299787, @Meinersbur wrote:

Should there be some documentation about the new attribute?

Other than the LangRef changes, where else do you think we should document this? I wasn't/am not aware of any other places where this should be documented.

[atmnpatel]

Rebasing.

llvm/include/llvm/Bitcode/LLVMBitCodes.h

enum AttributeKindCodes {
   ATTR_KIND_NOFREE = 62,
   ATTR_KIND_NOSYNC = 63,
   ATTR_KIND_SANITIZE_MEMTAG = 64,
   ATTR_KIND_PREALLOCATED = 65,
   ATTR_KIND_NO_MERGE = 66,
   ATTR_KIND_NULL_POINTER_IS_VALID = 67,
   ATTR_KIND_NOUNDEF = 68,
   ATTR_KIND_BYREF = 69,
+  ATTR_KIND_MUSTPROGRESS = 70,
 };
 
 enum ComdatSelectionKindCodes {
   COMDAT_SELECTION_KIND_ANY = 1,
   COMDAT_SELECTION_KIND_EXACT_MATCH = 2,
   COMDAT_SELECTION_KIND_LARGEST = 3,
   COMDAT_SELECTION_KIND_NO_DUPLICATES = 4,
   COMDAT_SELECTION_KIND_SAME_SIZE = 5,

llvm/include/llvm/IR/Attributes.td

 def WillReturn : EnumAttr<"willreturn">;
 
 /// Function only writes to memory.
 def WriteOnly : EnumAttr<"writeonly">;
 
 /// Zero extended before/after call.
 def ZExt : EnumAttr<"zeroext">;
 
+/// Function is required to make Forward Progress.
+def MustProgress : TypeAttr<"mustprogress">;
+
 /// Target-independent string attributes.
 def LessPreciseFPMAD : StrBoolAttr<"less-precise-fpmad">;
 def NoInfsFPMath : StrBoolAttr<"no-infs-fp-math">;
 def NoNansFPMath : StrBoolAttr<"no-nans-fp-math">;
 def NoSignedZerosFPMath : StrBoolAttr<"no-signed-zeros-fp-math">;
 def UnsafeFPMath : StrBoolAttr<"unsafe-fp-math">;
 def NoJumpTables : StrBoolAttr<"no-jump-tables">;
 def NoInlineLineTables : StrBoolAttr<"no-inline-line-tables">;

llvm/include/llvm/IR/Function.h

public:
   /// indirectly.
   bool doesNotRecurse() const {
     return hasFnAttribute(Attribute::NoRecurse);
   }
   void setDoesNotRecurse() {
     addFnAttr(Attribute::NoRecurse);
   }
 
+  /// Determine if the function is required to make forward progress.
+  bool mustProgress() const {
+    return hasFnAttribute(Attribute::MustProgress) ||
+           hasFnAttribute(Attribute::WillReturn);
+  }
+  void setMustProgress() { addFnAttr(Attribute::MustProgress); }
+
   /// True if the ABI mandates (or the user requested) that this
   /// function be in a unwind table.
   bool hasUWTable() const {
     return hasFnAttribute(Attribute::UWTable);
   }
   void setHasUWTable() {
     addFnAttr(Attribute::UWTable);
   }

llvm/lib/AsmParser/LLLexer.cpp

#define KEYWORD(STR)                                                           \
   KEYWORD(swifterror);
   KEYWORD(swiftself);
   KEYWORD(uwtable);
   KEYWORD(willreturn);
   KEYWORD(writeonly);
   KEYWORD(zeroext);
   KEYWORD(immarg);
   KEYWORD(byref);
+  KEYWORD(mustprogress);
 
   KEYWORD(type);
   KEYWORD(opaque);
 
   KEYWORD(comdat);
 
   // Comdat types
   KEYWORD(any);

llvm/lib/AsmParser/LLParser.cpp

while (true) {
     case lltok::kw_convergent: B.addAttribute(Attribute::Convergent); break;
     case lltok::kw_inaccessiblememonly:
       B.addAttribute(Attribute::InaccessibleMemOnly); break;
     case lltok::kw_inaccessiblemem_or_argmemonly:
       B.addAttribute(Attribute::InaccessibleMemOrArgMemOnly); break;
     case lltok::kw_inlinehint: B.addAttribute(Attribute::InlineHint); break;
     case lltok::kw_jumptable: B.addAttribute(Attribute::JumpTable); break;
     case lltok::kw_minsize: B.addAttribute(Attribute::MinSize); break;
+    case lltok::kw_mustprogress:
+      B.addAttribute(Attribute::MustProgress);
+      break;
     case lltok::kw_naked: B.addAttribute(Attribute::Naked); break;
     case lltok::kw_nobuiltin: B.addAttribute(Attribute::NoBuiltin); break;
     case lltok::kw_noduplicate: B.addAttribute(Attribute::NoDuplicate); break;
     case lltok::kw_nofree: B.addAttribute(Attribute::NoFree); break;
     case lltok::kw_noimplicitfloat:
       B.addAttribute(Attribute::NoImplicitFloat); break;
     case lltok::kw_noinline: B.addAttribute(Attribute::NoInline); break;
     case lltok::kw_nonlazybind: B.addAttribute(Attribute::NonLazyBind); break;
case lltok::kw_preallocated: {
       Type *Ty;
       if (ParsePreallocated(Ty))
         return true;
       B.addPreallocatedAttr(Ty);
       break;
     }
 
     // Error handling.
     case lltok::kw_inreg:
     case lltok::kw_signext:
     case lltok::kw_zeroext:
       HaveError |=
         Error(Lex.getLoc(),
               "invalid use of attribute on a function");
       break;
     case lltok::kw_byval:
     case lltok::kw_dereferenceable:
while (true) {
 
     case lltok::kw_alignstack:
     case lltok::kw_alwaysinline:
     case lltok::kw_argmemonly:
     case lltok::kw_builtin:
     case lltok::kw_inlinehint:
     case lltok::kw_jumptable:
     case lltok::kw_minsize:
+    case lltok::kw_mustprogress:
     case lltok::kw_naked:
     case lltok::kw_nobuiltin:
     case lltok::kw_noduplicate:
     case lltok::kw_noimplicitfloat:
     case lltok::kw_noinline:
     case lltok::kw_nonlazybind:
     case lltok::kw_nomerge:
     case lltok::kw_noredzone:
while (true) {
     case lltok::kw_sspreq:
     case lltok::kw_sspstrong:
     case lltok::kw_safestack:
     case lltok::kw_shadowcallstack:
     case lltok::kw_strictfp:
     case lltok::kw_uwtable:
       HaveError |= Error(Lex.getLoc(), "invalid use of function-only attribute");
       break;
     }
 
     Lex.Lex();
   }
 }
 
 /// ParseOptionalReturnAttrs - Parse a potentially empty list of return attributes.
 bool LLParser::ParseOptionalReturnAttrs(AttrBuilder &B) {
   bool HaveError = false;
while (true) {
     case lltok::kw_alignstack:
     case lltok::kw_alwaysinline:
     case lltok::kw_argmemonly:
     case lltok::kw_builtin:
     case lltok::kw_cold:
     case lltok::kw_inlinehint:
     case lltok::kw_jumptable:
     case lltok::kw_minsize:
+    case lltok::kw_mustprogress:
     case lltok::kw_naked:
     case lltok::kw_nobuiltin:
     case lltok::kw_noduplicate:
     case lltok::kw_noimplicitfloat:
     case lltok::kw_noinline:
     case lltok::kw_nonlazybind:
     case lltok::kw_nomerge:
     case lltok::kw_noredzone:
while (true) {
     case lltok::kw_preallocated:
       HaveError |=
           Error(Lex.getLoc(),
                 "invalid use of parameter-only/call site-only attribute");
       break;
     }
 
     Lex.Lex();
   }
 }
 
 static unsigned parseOptionalLinkageAux(lltok::Kind Kind, bool &HasLinkage) {
   HasLinkage = true;
   switch (Kind) {
   default:
     HasLinkage = false;
     return GlobalValue::ExternalLinkage;

llvm/lib/AsmParser/LLToken.h

enum Kind {
   kw_swifterror,
   kw_swiftself,
   kw_uwtable,
   kw_willreturn,
   kw_writeonly,
   kw_zeroext,
   kw_immarg,
   kw_byref,
+  kw_mustprogress,
 
   kw_type,
   kw_opaque,
 
   kw_comdat,
 
   // Comdat types
   kw_any,

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
   case bitc::ATTR_KIND_SANITIZE_MEMTAG:
     return Attribute::SanitizeMemTag;
   case bitc::ATTR_KIND_PREALLOCATED:
     return Attribute::Preallocated;
   case bitc::ATTR_KIND_NOUNDEF:
     return Attribute::NoUndef;
   case bitc::ATTR_KIND_BYREF:
     return Attribute::ByRef;
+  case bitc::ATTR_KIND_MUSTPROGRESS:
+    return Attribute::MustProgress;
   }
 }
 
 Error BitcodeReader::parseAlignmentValue(uint64_t Exponent,
                                          MaybeAlign &Alignment) {
   // Note: Alignment in bitcode files is incremented by 1, so that zero
   // can be used for default alignment.
   if (Exponent > Value::MaxAlignmentExponent + 1)

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   case Attribute::SanitizeMemTag:
     return bitc::ATTR_KIND_SANITIZE_MEMTAG;
   case Attribute::Preallocated:
     return bitc::ATTR_KIND_PREALLOCATED;
   case Attribute::NoUndef:
     return bitc::ATTR_KIND_NOUNDEF;
   case Attribute::ByRef:
     return bitc::ATTR_KIND_BYREF;
+  case Attribute::MustProgress:
+    return bitc::ATTR_KIND_MUSTPROGRESS;
   case Attribute::EndAttrKinds:
     llvm_unreachable("Can not encode end-attribute kinds marker.");
   case Attribute::None:
     llvm_unreachable("Can not encode none-attribute.");
   case Attribute::EmptyKey:
   case Attribute::TombstoneKey:
     llvm_unreachable("Trying to encode EmptyKey/TombstoneKey");
   }

llvm/lib/IR/Attributes.cpp

std::string Attribute::getAsString(bool InAttrGrp) const {
   if (hasAttribute(Attribute::ZExt))
     return "zeroext";
   if (hasAttribute(Attribute::Cold))
     return "cold";
   if (hasAttribute(Attribute::ImmArg))
     return "immarg";
   if (hasAttribute(Attribute::NoUndef))
     return "noundef";
+  if (hasAttribute(Attribute::MustProgress))
+    return "mustprogress";
 
   const bool IsByVal = hasAttribute(Attribute::ByVal);
   if (IsByVal || hasAttribute(Attribute::StructRet)) {
     std::string Result;
     Result += IsByVal ? "byval" : "sret";
     if (Type *Ty = getValueAsType()) {
       raw_string_ostream OS(Result);
       Result += '(';

llvm/lib/IR/Verifier.cpp

static bool isFuncOnlyAttr(Attribute::AttrKind Kind) {
   case Attribute::NoRecurse:
   case Attribute::InaccessibleMemOnly:
   case Attribute::InaccessibleMemOrArgMemOnly:
   case Attribute::AllocSize:
   case Attribute::SpeculativeLoadHardening:
   case Attribute::Speculatable:
   case Attribute::StrictFP:
   case Attribute::NullPointerIsValid:
+  case Attribute::MustProgress:
     return true;
   default:
     break;
   }
   return false;
 }
 
 /// Return true if this is a function attribute that can also appear on

llvm/lib/Transforms/Utils/CodeExtractor.cpp

if (Attr.isStringAttribute()) {
       case Attribute::SanitizeMemTag:
       case Attribute::SpeculativeLoadHardening:
       case Attribute::StackProtect:
       case Attribute::StackProtectReq:
       case Attribute::StackProtectStrong:
       case Attribute::StrictFP:
       case Attribute::UWTable:
       case Attribute::NoCfCheck:
+      case Attribute::MustProgress:
         break;
       }
 
     newFunction->addFnAttr(Attr);
   }
   newFunction->getBasicBlockList().push_back(newRootNode);
 
   // Create an iterator to name all of the arguments we inserted.

llvm/test/Assembler/mustprogress-parse-error-0.ll

+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+; CHECK: [[@LINE+1]]:35: error: invalid use of function-only attribute 
+define void @test_mustprogress(i8 mustprogress %a) {
+  ret void
+}
+

llvm/test/Assembler/mustprogress-parse-error-1.ll

+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+; CHECK: [[@LINE+1]]:8: error: invalid use of function-only attribute 
+define mustprogress void @test_mustprogress(i8 %a) {
+  ret void
+}
+

llvm/test/Assembler/mustprogress-parse-error-2.ll

+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+; CHECK: [[@LINE+1]]:52: error: expected '{' in function body
+define i32* @test_mustprogress(i8 %a) mustprogress 8 {
+  ret void
+}
+

llvm/test/Bitcode/attributes.ll

 }
 
 ; CHECK: define void @f67(i32* byref(i32) %a)
 define void @f67(i32* byref(i32) %a)
 {
   ret void
 }
 
+; CHECK: define void @f68() #41
+define void @f68() mustprogress
+{
+  ret void
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
 ; CHECK: attributes #3 = { readonly }
 ; CHECK: attributes #4 = { noinline }
 ; CHECK: attributes #5 = { alwaysinline }
 ; CHECK: attributes #6 = { optsize }
 ; CHECK: attributes #7 = { ssp }

 ; CHECK: attributes #33 = { speculatable }
 ; CHECK: attributes #34 = { sanitize_hwaddress }
 ; CHECK: attributes #35 = { shadowcallstack }
 ; CHECK: attributes #36 = { willreturn }
 ; CHECK: attributes #37 = { nofree }
 ; CHECK: attributes #38 = { nosync }
 ; CHECK: attributes #39 = { sanitize_memtag }
 ; CHECK: attributes #40 = { null_pointer_is_valid }
+; CHECK: attributes #41 = { mustprogress }
 ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin }