[Attributes] Add IntrinsicLoweredToCall attribute.

[mattd]

Summary

Many intrinsics are not lowered to a function call; however, there is currently no way of knowing whether any given intrinsic is lowered to a call.
The function TargetTransformInfoImpl::isLoweredToCall was originally implemented to return 'false' for all intrinsics, suggesting that there is not the case
of any intrinsic being lowered to a call. The patch introduced here adds an attribute to indicate that a particular intrinsic will be lowered to a call instruction.
The aforementioned function has been updated to check the IsLoweredToCall attribute when it encounters an intrinsic.

This patch was inspired by the discussion at the following URL:
https://reviews.llvm.org/D41104

The semantics of lowering is preserved with this change, but I anticipate,
in the future, some intrinsics might be added or updated to reflect that they
truly be come a call.

[efriedma]

I don't IntrLoweredToCall is the attribute you want.

There are some intrinsics which are essentially always lowered to calls. But there are a lot of intrinsics which may or may not be lowered to a call, depending on the exact capabilities of the target. For example, llvm.trunc.f32() is lowered to a single instruction on some processor variants, and a call to trunc() on others. So it's not a fundamental property of the intrinsic; we have to actually query the target. (Getting this right would probably be helpful for the inliner.)

That said, that isn't what you want when you're trying to deduce norecurse; you don't actually care whether the intrinsic is lowered to a call instruction, just whether the lowering calls another user-defined function.

[mattd]

Thanks for the feedback @eli.friedman.

Aside from the norecurse cases, there are other places that could probably benefit from knowing if a particular intrinsic is either a call to some user code, just an instruction, or metadata. As you mentioned, this is target specific. I think it makes sense to make isLoweredToCall a virtual function in TargetLowering. Each target-specific implementation of TargetLowering could override that routine to handle specific instructions that may/may-not be lowered to a user call. As an initial pass we could take the routine as it is now and make that the default behavior, which doesn't change functionality, but opens the doors for other targets to make use of this functionality later.

[efriedma]

Yes, isLoweredToCall() should probably be virtual, so targets can handle target-specific intrinsics. We should probably also add more target-independent handling (llvm.dbg.value is never a call, llvm.trunc isn't a call if ISD::FTRUNC is legal, etc.). The attribute doesn't really make sense.

[efriedma]

Sorry, had to refresh my memory about how TargetTransformInfo is implemented. isLoweredToCall is already virtual; TargetTransformInfo is just using some some funny template magic to reduce the abstraction penalty. You can override it by adding an implementation to BasicTTIImpl.h/ARMTargetTransformInfo.h/etc.

[mattd]

Thanks again for the reply, much appreciated! I suppose there is nothing that really needs to change with how the isLoweredToCall predicate is presented to the rest of the codebase. At one point I wanted to move it from TTI to TargetLowering (TLI), because the predicate is about lowering semantics, and it seemed to make sense that isLoweredToCall could live there too. TargetLowering doesn't rely on the CRTP like TTI, so if we did move the predicate, we'd probably make it virtual. But for now, I don't see an advantage in moving it; however, thanks for pointing out the template magic, CRTP, and I'm now convinced that moving the predicate isn't really necessary, in other words it can still be overriden by a target implementation based on TTI.

Regarding the attribute: I was a bit biased towards implementing this functionality as an attribute, given discussion on the previous review that brought the limitation of isLoweredToCall to my attention: https://reviews.llvm.org/D41104. However, you put forward a good point regarding 'trunc.' and a handful of other intrinsics.

Let me step back for a minute and more generally address what I am trying to solve, in the norecurse case, and for potentially other cases as well. What I really want to know is if an intrinsic is just metadata or not. If the intrinsic is only an "information passer" used for communicating with other parts of the compiler (e.g., dbg.value, lifetime.start, etc). While that will not solve isLoweredToCall, I wouldn't necessarily need to use isLoweredToCall in the cases I am interested in. I believe this will solve the issue with norecurse making decisions on 'metadata' intrinsics (llvm.dbg.). If adding an attribute to denote that an intrinsic will only be used as metadata sounds like a reasonable change, then I'd be happy to rename the changes in this patch to something more appropriate, such as: "Attribute::MetadataIntrinsic", and then update the obvious metadata specific intrinsics in Intrinsics.td.

[efriedma]

I feel like we've been going around in circles. You're been repeatedly coming up with various distinctions which are not actually the distinction you want, specifically that you can infer norecurse for functions which call a given intrinsic, i.e. whether there is a potential control flow path from a particular callsite to a particular function. This is not whether the intrinsic is lowered to a call instruction, this is not whether an intrinsic is "metadata".

[mattd]

In D45216#1058779, @efriedma wrote:

This is not whether the intrinsic is lowered to a call instruction, this is not whether an intrinsic is "metadata".

Correct, but having that data can be useful. I'm not trying to necessarily
change how norecurse is decided, rather I just want to prevent
certain intrinsics from having an influence. What I envisioned was
to use such a "is metadata" attribute to completely ignore intrinsics
that we know should not have an influence on norecurse. There are
other cases in the compiler where I'd also like to have this
"is metadata" information.

For norecurse, I would think that checking such an "is metadata"
attribute in the for-loop of the following function would help prevent
debug intrinsics from influencing the placement of norecurse
attributes on a function:
https://llvm.org/doxygen/FunctionAttrs_8cpp_source.html#l01260

for (Instruction &I : instructions(*F))
  if (auto CS = CallSite(&I)) {
    Function *Callee = CS.getCalledFunction();
    if (Callee && Callee->isAMetadataIntrinsic())  // Just an idea...
      continue;
    if (!Callee || Callee == F || !Callee->doesNotRecurse())
      // Function calls a potentially recursive function.
      return false;
  }

[efriedma]

We have isa<DbgInfoIntrinsic> to check for all debug info.

More generally, I'm not sure there's a definition of "metadata" that would suit enough passes to make it worthwhile. There's a function for estimating the code size of a call: getIntrinsicCost in TTI returns TCC_Free for a bunch of intrinsics. llvm::isAssumeLikeIntrinsic checks... well, I'm actually not quite sure what it checks; it currently looks for metadata-ish intrinsics, but the documentation for it doesn't really match that meaning. And we have isLoweredToCall, which you were looking into changing. So I guess there's demand for something, but I'm not quite sure what it looks like.

include/llvm/Analysis/TargetTransformInfoImpl.h

bool isLoweredToCall(const Function *F) {
     assert(F && "A concrete function must be provided to this routine.");
 
     // FIXME: These should almost certainly not be handled here, and instead
     // handled with the help of TLI or the target itself. This was largely
     // ported from existing analysis heuristics here so that such refactorings
     // can take place in the future.
 
     if (F->isIntrinsic())
-      return false;
+      return F->hasFnAttribute(Attribute::IntrinsicLoweredToCall);
 
     if (F->hasLocalLinkage() || !F->hasName())
       return true;
 
     StringRef Name = F->getName();
 
     // These will all likely lower to a single selection DAG node.
     if (Name == "copysign" || Name == "copysignf" || Name == "copysignl" ||

include/llvm/Bitcode/LLVMBitCodes.h

enum AttributeKindCodes {
   ATTR_KIND_INACCESSIBLEMEM_OR_ARGMEMONLY = 50,
   ATTR_KIND_ALLOC_SIZE = 51,
   ATTR_KIND_WRITEONLY = 52,
   ATTR_KIND_SPECULATABLE = 53,
   ATTR_KIND_STRICT_FP = 54,
   ATTR_KIND_SANITIZE_HWADDRESS = 55,
   ATTR_KIND_NOCF_CHECK = 56,
   ATTR_KIND_OPT_FOR_FUZZING = 57,
+  ATTR_KIND_INTRINSIC_LOWERED_TO_CALL = 58,
 };
 
 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,

include/llvm/IR/Attributes.td

 def InAlloca : EnumAttr<"inalloca">;
 
 /// Source said inlining was desirable.
 def InlineHint : EnumAttr<"inlinehint">;
 
 /// Force argument to be passed in register.
 def InReg : EnumAttr<"inreg">;
 
+/// Treat this function like an intrinsic that is lowered to a function call.
+def IntrinsicLoweredToCall : EnumAttr<"intrinsic_lowered_to_call">;
+
 /// Build jump-instruction tables and replace refs.
 def JumpTable : EnumAttr<"jumptable">;
 
 /// Function must be optimized for size first.
 def MinSize : EnumAttr<"minsize">;
 
 /// Naked function.
 def Naked : EnumAttr<"naked">;

include/llvm/IR/Intrinsics.td

 // language of the IntrNoMem, IntrReadMem, IntrWriteMem, and IntrArgMemOnly
 // intrinsic properties.  By default, intrinsics are assumed to have side
 // effects, so this property is only necessary if you have defined one of
 // the memory properties listed above.
 // For this property, 'side effects' has the same meaning as 'side effects'
 // defined by the hasSideEffects property of the TableGen Instruction class.
 def IntrHasSideEffects : IntrinsicProperty;
 
+// This property indicates that the associated intrinsic is lowered to a
+// function call.
+def IntrLoweredToCall : IntrinsicProperty;
+
 //===----------------------------------------------------------------------===//
 // Types used by intrinsics.
 //===----------------------------------------------------------------------===//
 
 class LLVMType<ValueType vt> {
   ValueType VT = vt;
 }
 

lib/AsmParser/LLLexer.cpp

#define KEYWORD(STR)                                                           \
   KEYWORD(cold);
   KEYWORD(convergent);
   KEYWORD(dereferenceable);
   KEYWORD(dereferenceable_or_null);
   KEYWORD(inaccessiblememonly);
   KEYWORD(inaccessiblemem_or_argmemonly);
   KEYWORD(inlinehint);
   KEYWORD(inreg);
+  KEYWORD(intrinsic_lowered_to_call);
   KEYWORD(jumptable);
   KEYWORD(minsize);
   KEYWORD(naked);
   KEYWORD(nest);
   KEYWORD(noalias);
   KEYWORD(nobuiltin);
   KEYWORD(nocapture);
   KEYWORD(noduplicate);

lib/AsmParser/LLParser.cpp

while (true) {
     case lltok::kw_builtin: B.addAttribute(Attribute::Builtin); break;
     case lltok::kw_cold: B.addAttribute(Attribute::Cold); break;
     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_intrinsic_lowered_to_call:
+      B.addAttribute(Attribute::IntrinsicLoweredToCall);
+      break;
     case lltok::kw_jumptable: B.addAttribute(Attribute::JumpTable); break;
     case lltok::kw_minsize: B.addAttribute(Attribute::MinSize); 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_noimplicitfloat:
       B.addAttribute(Attribute::NoImplicitFloat); break;
     case lltok::kw_noinline: B.addAttribute(Attribute::NoInline); break;

lib/AsmParser/LLToken.h

enum Kind {
   kw_cold,
   kw_convergent,
   kw_dereferenceable,
   kw_dereferenceable_or_null,
   kw_inaccessiblememonly,
   kw_inaccessiblemem_or_argmemonly,
   kw_inlinehint,
   kw_inreg,
+  kw_intrinsic_lowered_to_call,
   kw_jumptable,
   kw_minsize,
   kw_naked,
   kw_nest,
   kw_noalias,
   kw_nobuiltin,
   kw_nocapture,
   kw_noduplicate,

lib/Bitcode/Reader/BitcodeReader.cpp

static uint64_t getRawAttributeMask(Attribute::AttrKind Val) {
   case Attribute::SwiftSelf:       return 1ULL << 51;
   case Attribute::SwiftError:      return 1ULL << 52;
   case Attribute::WriteOnly:       return 1ULL << 53;
   case Attribute::Speculatable:    return 1ULL << 54;
   case Attribute::StrictFP:        return 1ULL << 55;
   case Attribute::SanitizeHWAddress: return 1ULL << 56;
   case Attribute::NoCfCheck:       return 1ULL << 57;
   case Attribute::OptForFuzzing:   return 1ULL << 58;
+  case Attribute::IntrinsicLoweredToCall: return 1ULL << 59;
   case Attribute::Dereferenceable:
     llvm_unreachable("dereferenceable attribute not supported in raw format");
     break;
   case Attribute::DereferenceableOrNull:
     llvm_unreachable("dereferenceable_or_null attribute not supported in raw "
                      "format");
     break;
   case Attribute::ArgMemOnly:
static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
   case bitc::ATTR_KIND_INACCESSIBLEMEM_ONLY:
     return Attribute::InaccessibleMemOnly;
   case bitc::ATTR_KIND_INACCESSIBLEMEM_OR_ARGMEMONLY:
     return Attribute::InaccessibleMemOrArgMemOnly;
   case bitc::ATTR_KIND_INLINE_HINT:
     return Attribute::InlineHint;
   case bitc::ATTR_KIND_IN_REG:
     return Attribute::InReg;
+  case bitc::ATTR_KIND_INTRINSIC_LOWERED_TO_CALL:
+    return Attribute::IntrinsicLoweredToCall;
   case bitc::ATTR_KIND_JUMP_TABLE:
     return Attribute::JumpTable;
   case bitc::ATTR_KIND_MIN_SIZE:
     return Attribute::MinSize;
   case bitc::ATTR_KIND_NAKED:
     return Attribute::Naked;
   case bitc::ATTR_KIND_NEST:
     return Attribute::Nest;

lib/Bitcode/Writer/BitcodeWriter.cpp

static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   case Attribute::InaccessibleMemOnly:
     return bitc::ATTR_KIND_INACCESSIBLEMEM_ONLY;
   case Attribute::InaccessibleMemOrArgMemOnly:
     return bitc::ATTR_KIND_INACCESSIBLEMEM_OR_ARGMEMONLY;
   case Attribute::InlineHint:
     return bitc::ATTR_KIND_INLINE_HINT;
   case Attribute::InReg:
     return bitc::ATTR_KIND_IN_REG;
+  case Attribute::IntrinsicLoweredToCall:
+    return bitc::ATTR_KIND_INTRINSIC_LOWERED_TO_CALL;
   case Attribute::JumpTable:
     return bitc::ATTR_KIND_JUMP_TABLE;
   case Attribute::MinSize:
     return bitc::ATTR_KIND_MIN_SIZE;
   case Attribute::Naked:
     return bitc::ATTR_KIND_NAKED;
   case Attribute::Nest:
     return bitc::ATTR_KIND_NEST;

lib/IR/Attributes.cpp

std::string Attribute::getAsString(bool InAttrGrp) const {
   if (hasAttribute(Attribute::InaccessibleMemOrArgMemOnly))
     return "inaccessiblemem_or_argmemonly";
   if (hasAttribute(Attribute::InAlloca))
     return "inalloca";
   if (hasAttribute(Attribute::InlineHint))
     return "inlinehint";
   if (hasAttribute(Attribute::InReg))
     return "inreg";
+  if (hasAttribute(Attribute::IntrinsicLoweredToCall))
+    return "intrinsic_lowered_to_call";
   if (hasAttribute(Attribute::JumpTable))
     return "jumptable";
   if (hasAttribute(Attribute::MinSize))
     return "minsize";
   if (hasAttribute(Attribute::Naked))
     return "naked";
   if (hasAttribute(Attribute::Nest))
     return "nest";

lib/IR/Verifier.cpp

static bool isFuncOnlyAttr(Attribute::AttrKind Kind) {
   case Attribute::OptForFuzzing:
   case Attribute::OptimizeNone:
   case Attribute::JumpTable:
   case Attribute::Convergent:
   case Attribute::ArgMemOnly:
   case Attribute::NoRecurse:
   case Attribute::InaccessibleMemOnly:
   case Attribute::InaccessibleMemOrArgMemOnly:
+  case Attribute::IntrinsicLoweredToCall:
   case Attribute::AllocSize:
   case Attribute::Speculatable:
   case Attribute::StrictFP:
     return true;
   default:
     break;
   }
   return false;

lib/Transforms/Utils/CodeExtractor.cpp

if (Attr.isStringAttribute()) {
       case Attribute::ByVal:
       case Attribute::Convergent:
       case Attribute::Dereferenceable:
       case Attribute::DereferenceableOrNull:
       case Attribute::InAlloca:
       case Attribute::InReg:
       case Attribute::InaccessibleMemOnly:
       case Attribute::InaccessibleMemOrArgMemOnly:
+      case Attribute::IntrinsicLoweredToCall:
       case Attribute::JumpTable:
       case Attribute::Naked:
       case Attribute::Nest:
       case Attribute::NoAlias:
       case Attribute::NoBuiltin:
       case Attribute::NoCapture:
       case Attribute::NoReturn:
       case Attribute::None:

test/Bitcode/attributes.ll

 }
 
 declare void @nobuiltin()
 
 define void @f34()
 ; CHECK: define void @f34()
 {
         call void @nobuiltin() nobuiltin
-; CHECK: call void @nobuiltin() #35
+; CHECK: call void @nobuiltin() #36
         ret void;
 }
 
 define void @f35() optnone noinline
 ; CHECK: define void @f35() #23
 {
         ret void;
 }

 }
 
 ; CHECK: define void @f58() #34
 define void @f58() sanitize_hwaddress
 {
         ret void;
 }
 
+; CHECK: define void @f59() #35
+define void @f59() intrinsic_lowered_to_call
+{
+        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 #27 = { norecurse }
 ; CHECK: attributes #28 = { inaccessiblememonly }
 ; CHECK: attributes #29 = { inaccessiblemem_or_argmemonly }
 ; CHECK: attributes #30 = { allocsize(0) }
 ; CHECK: attributes #31 = { allocsize(0,1) }
 ; CHECK: attributes #32 = { writeonly }
 ; CHECK: attributes #33 = { speculatable }
 ; CHECK: attributes #34 = { sanitize_hwaddress }
-; CHECK: attributes #35 = { nobuiltin }
+; CHECK: attributes #35 = { intrinsic_lowered_to_call }
+; CHECK: attributes #36 = { nobuiltin }

utils/TableGen/CodeGenIntrinsics.h

struct CodeGenIntrinsic {
 
   /// True if the intrinsic has side effects that aren't captured by any
   /// of the other flags.
   bool hasSideEffects;
 
   // True if the intrinsic is marked as speculatable.
   bool isSpeculatable;
 
+  /// True if the intrinsic is lowered to a function call.
+  bool isLoweredToCall;
+
   enum ArgAttribute { NoCapture, Returned, ReadOnly, WriteOnly, ReadNone };
   std::vector<std::pair<unsigned, ArgAttribute>> ArgumentAttributes;
 
   bool hasProperty(enum SDNP Prop) const {
     return Properties & (1 << Prop);
   }
 
   CodeGenIntrinsic(Record *R);

utils/TableGen/CodeGenTarget.cpp

CodeGenIntrinsic::CodeGenIntrinsic(Record *R) {
   isOverloaded = false;
   isCommutative = false;
   canThrow = false;
   isNoReturn = false;
   isNoDuplicate = false;
   isConvergent = false;
   isSpeculatable = false;
   hasSideEffects = false;
+  isLoweredToCall = false;
 
   if (DefName.size() <= 4 ||
       std::string(DefName.begin(), DefName.begin() + 4) != "int_")
     PrintFatalError("Intrinsic '" + DefName + "' does not start with 'int_'!");
 
   EnumName = std::string(DefName.begin()+4, DefName.end());
 
   if (R->getValue("GCCBuiltinName"))  // Ignore a missing GCCBuiltinName field.
for (unsigned i = 0, e = PropList->size(); i != e; ++i) {
     else if (Property->getName() == "IntrConvergent")
       isConvergent = true;
     else if (Property->getName() == "IntrNoReturn")
       isNoReturn = true;
     else if (Property->getName() == "IntrSpeculatable")
       isSpeculatable = true;
     else if (Property->getName() == "IntrHasSideEffects")
       hasSideEffects = true;
+    else if (Property->getName() == "IntrLoweredToCall")
+      isLoweredToCall = true;
     else if (Property->isSubClassOf("NoCapture")) {
       unsigned ArgNo = Property->getValueAsInt("ArgNo");
       ArgumentAttributes.push_back(std::make_pair(ArgNo, NoCapture));
     } else if (Property->isSubClassOf("Returned")) {
       unsigned ArgNo = Property->getValueAsInt("ArgNo");
       ArgumentAttributes.push_back(std::make_pair(ArgNo, Returned));
     } else if (Property->isSubClassOf("ReadOnly")) {
       unsigned ArgNo = Property->getValueAsInt("ArgNo");

utils/TableGen/IntrinsicEmitter.cpp

if (L->isConvergent != R->isConvergent)
       return R->isConvergent;
 
     if (L->isSpeculatable != R->isSpeculatable)
       return R->isSpeculatable;
 
     if (L->hasSideEffects != R->hasSideEffects)
       return R->hasSideEffects;
 
+    if (L->isLoweredToCall != R->isLoweredToCall)
+      return R->isLoweredToCall;
+
     // Try to order by readonly/readnone attribute.
     CodeGenIntrinsic::ModRefBehavior LK = L->ModRef;
     CodeGenIntrinsic::ModRefBehavior RK = R->ModRef;
     if (LK != RK) return (LK > RK);
 
     // Order by argument attributes.
     // This is reliable because each side is already sorted internally.
     return (L->ArgumentAttributes < R->ArgumentAttributes);
if (!intrinsic.canThrow ||
         addComma = true;
       }
       if (intrinsic.isSpeculatable) {
         if (addComma)
           OS << ",";
         OS << "Attribute::Speculatable";
         addComma = true;
       }
+      if (intrinsic.isLoweredToCall) {
+        if (addComma)
+          OS << ",";
+        OS << "Attribute::IntrinsicLoweredToCall";
+        addComma = true;
+      }
 
       switch (intrinsic.ModRef) {
       case CodeGenIntrinsic::NoMem:
         if (addComma)
           OS << ",";
         OS << "Attribute::ReadNone";
         break;
       case CodeGenIntrinsic::ReadArgMem: