[TLI] Optimize no-builtins attribute check (NFC)

[nikic]

Summary

Looking up a string function attribute is expensive, because attributes are stored as a plain list (not a map), and functions tend to have a large number of attributes. As the TargetLibraryInfo constructor needs to scan over all attributes to check for no-builtin-* anyway, we can also integrate the no-builtins check in the same loop. D77632 will then be able to also check for veclib in the same loop, which should mitigate most of the negative impact of that change.

Here are the improvements to instructions retired with this change: http://llvm-compile-time-tracker.com/compare.php?from=b3f5472c2b9c8cf99239a9ac655555e9f0ba9e5d&to=b1eb5ae2f8bcdb888b8a0d7c0356fa10e6e51d6a&stat=instructions

[arsenm]

I do think we need to do something about string attributes in general

[tejohnson]

Nice find/fix. Lgtm

include/llvm/Analysis/TargetLibraryInfo.h

236

You could break out of the loop in this case. Although that won't work any more if this loop is extended to look for veclib unfortunately.

[wenlei]

Thanks for fix. On the other hand, if hasFnAttribute is so expensive to the point that we need to refrain from using it, that's probably an indication that we should try to speed up hasFnAttribute.

because attributes are stored as a plain list (not a map)

And sounds like there's opportunity for a nice speed up.

[nikic]

I've put up D78859 for the alternative suggestion to make string attribute lookup cheaper in general. This turned out to be super simple to implement and makes for a major compile-time saving (~3%), so that's probably better than this patch :)

[nikic]

Abandoning this one, as I expect this to be cheap enough after D78859.

include/llvm/Analysis/TargetLibraryInfo.h

class TargetLibraryInfo {
   BitVector OverrideAsUnavailable;
 
 public:
   explicit TargetLibraryInfo(const TargetLibraryInfoImpl &Impl,
                              Optional<const Function *> F = None)
       : Impl(&Impl), OverrideAsUnavailable(NumLibFuncs) {
     if (!F)
       return;
-    if ((*F)->hasFnAttribute("no-builtins"))
-      disableAllFunctions();
-    else {
-      // Disable individual libc/libm calls in TargetLibraryInfo.
-      LibFunc LF;
-      AttributeSet FnAttrs = (*F)->getAttributes().getFnAttributes();
-      for (const Attribute &Attr : FnAttrs) {
-        if (!Attr.isStringAttribute())
-          continue;
-        auto AttrStr = Attr.getKindAsString();
-        if (!AttrStr.consume_front("no-builtin-"))
-          continue;
+
+      if (AttrStr == "no-builtins") {
+        disableAllFunctions();
+      } else if (AttrStr.consume_front("no-builtin-")) {
+        // Disable individual libc/libm calls in TargetLibraryInfo.
+        LibFunc LF;
         if (getLibFunc(AttrStr, LF))
           setUnavailable(LF);
       }
     }
   }
 
   // Provide value semantics.
   TargetLibraryInfo(const TargetLibraryInfo &TLI)