[clang] Turn -fno-builtin flag into an IR Attribute

[gchatelet]

Summary

This is a follow up on https://reviews.llvm.org/D61634#1742154 to turn the clang driver -fno-builtin flag into an IR attribute.
I also investigated pushing the attribute earlier on (in Sema) but it looks like this patch is simple and will cover all function calls.

[aaron.ballman]

Adding a few more reviewers who may know more about LLVM attributes.

clang/lib/CodeGen/CGCall.cpp

1884

You can drop the top-level const here (we don't typically top-level const qualify locals).

1890–1891

llvm::for_each()?

1893–1894

llvm::for_each()?

[gchatelet]

• Address comments

[tejohnson]

This LGTM, although I have a couple of questions that are orthogonal to this patch and shouldn't block it. Please wait to see if @aaron.ballman has any more comments.

clang/test/CodeGen/libcalls-fno-builtin.c

163–164

Orthogonal to this patch:

Looks like there are 2 nobuiltin attributes now? AFAICT the old "nobuiltin" gets applied to any and all cases where either -fno-builtin or -fno-builtin-{name} applied. Is it obviated by the new attributes?

Also, why are the new ones quoted and the old one not?

[gchatelet]

clang/test/CodeGen/libcalls-fno-builtin.c

163–164

Here is my understanding so far:
There are two systems for attributes.

• In the original design attributes were boolean only and they would be packed in 64 bit integer. nobuiltin is one of these.
• Then the attribute system got extended to allow for more than 64 entries and also allow for types. Attributes are now key/value pairs where value can be bool, int or string. "nobuiltins" is a such boolean attribute (note the trailing s)

Now I wanted to create a different attribute than the original nobuiltin because semantic might be different.

Obviously this is not ideal.
There are two options AFAICT:

• conflate the two and reuse the original one to get a mode compact representation
• remove the original one and release one bit for other important attribute to use.

For the individual ones the list is open ended so there's no way we can retrofit it into the previous scheme.

@efriedma, @hfinkel do you have any suggestions or a better approach here?

[tejohnson]

clang/test/CodeGen/libcalls-fno-builtin.c

163–164

Thanks for the background. Yeah it seems like a good idea to remove the overlap here at some point, and the semantics do seem to be different at the moment. Like I said earlier, though, I don't think it should block this particular patch since the duplication predates it (and it also allows forward progress on the TLI side).

[gchatelet]

clang/test/CodeGen/libcalls-fno-builtin.c

163–164

SGTM I'll submit the patch then.

clang/lib/CodeGen/CGCall.cpp

if (const FunctionDecl *Fn = dyn_cast<FunctionDecl>(TargetDecl)) {
       const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Fn);
       const bool IsVirtualCall = MD && MD->isVirtual();
       // Don't use [[noreturn]], _Noreturn or [[no_builtin]] for a call to a
       // virtual function. These attributes are not inherited by overloads.
       if (!(AttrOnCallSite && IsVirtualCall)) {
         if (Fn->isNoReturn())
           FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
 
-        if (const auto *NBA = TargetDecl->getAttr<NoBuiltinAttr>()) {
-          bool HasWildcard = llvm::is_contained(NBA->builtinNames(), "*");
-          if (HasWildcard)
-            FuncAttrs.addAttribute("no-builtins");
-          else
-            for (StringRef BuiltinName : NBA->builtinNames()) {
-              SmallString<32> AttributeName;
-              AttributeName += "no-builtin-";
-              AttributeName += BuiltinName;
-              FuncAttrs.addAttribute(AttributeName);
-            }
+        const auto *NBA = Fn->getAttr<NoBuiltinAttr>();
+        bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
+        if (getLangOpts().NoBuiltin || HasWildcard)
+        else {
+          const auto AddNoBuiltinAttr = [&FuncAttrs](StringRef BuiltinName) {
+          };
+          for (StringRef BuiltinName : getLangOpts().NoBuiltinFuncs)
+            AddNoBuiltinAttr(BuiltinName);
+          if (NBA)
+            for (StringRef BuiltinName : NBA->builtinNames())
+              AddNoBuiltinAttr(BuiltinName);
         }
       }
     }
 
     // 'const', 'pure' and 'noalias' attributed functions are also nounwind.
     if (TargetDecl->hasAttr<ConstAttr>()) {
       FuncAttrs.addAttribute(llvm::Attribute::ReadNone);
       FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);

clang/test/CodeGen/libcalls-fno-builtin.c

-// RUN: %clang_cc1 -S -emit-llvm -fno-builtin -o - %s | FileCheck %s
+// RUN: %clang_cc1 -S -emit-llvm -fno-builtin -o - %s | FileCheck --check-prefixes=GLOBAL,CHECK %s
 // RUN: %clang_cc1 -S -emit-llvm -fno-builtin-ceil -fno-builtin-copysign -fno-builtin-cos \
 // RUN:  -fno-builtin-fabs -fno-builtin-floor -fno-builtin-strcat -fno-builtin-strncat \
 // RUN:  -fno-builtin-strchr -fno-builtin-strrchr -fno-builtin-strcmp -fno-builtin-strncmp \
 // RUN:  -fno-builtin-strcpy -fno-builtin-stpcpy -fno-builtin-strncpy -fno-builtin-strlen \
 // RUN:  -fno-builtin-strpbrk -fno-builtin-strspn -fno-builtin-strtod -fno-builtin-strtof \
 // RUN:  -fno-builtin-strtold -fno-builtin-strtol -fno-builtin-strtoll -fno-builtin-strtoul \
 // RUN:  -fno-builtin-strtoull -fno-builtin-fread -fno-builtin-fwrite -fno-builtin-fopen \
-// RUN:  -o - %s | FileCheck %s
+// RUN:  -o - %s | FileCheck --check-prefixes=INDIVIDUAL,CHECK %s
 // RUN: %clang_cc1 -S -O3 -fno-builtin -o - %s | FileCheck --check-prefix=ASM %s
 // RUN: %clang_cc1 -S -O3 -fno-builtin-ceil -o - %s | FileCheck --check-prefix=ASM-INDIV %s
 
 // rdar://10551066
 
 typedef __SIZE_TYPE__ size_t;
 typedef struct FILE FILE;
 

 // ASM: t1
 // ASM: ceil
 
 // ASM-INDIV: t1
 // ASM-INDIV: ceil
 
 double t2(double x, double y) { return copysign(x,y); }
 // CHECK-LABEL: t2
-// CHECK: call{{.*}}@copysign{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@copysign{{.*}} #2
 
 double t3(double x) { return cos(x); }
 // CHECK-LABEL: t3
-// CHECK: call{{.*}}@cos{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@cos{{.*}} #2
 
 double t4(double x) { return fabs(x); }
 // CHECK-LABEL: t4
-// CHECK: call{{.*}}@fabs{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@fabs{{.*}} #2
 
 double t5(double x) { return floor(x); }
 // CHECK-LABEL: t5
-// CHECK: call{{.*}}@floor{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@floor{{.*}} #2
 
 char *t6(char *x) { return strcat(x, ""); }
 // CHECK-LABEL: t6
-// CHECK: call{{.*}}@strcat{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strcat{{.*}} #2
 
 char *t7(char *x) { return strncat(x, "", 1); }
 // CHECK-LABEL: t7
-// CHECK: call{{.*}}@strncat{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strncat{{.*}} #2
 
 char *t8(void) { return strchr("hello, world", 'w'); }
 // CHECK-LABEL: t8
-// CHECK: call{{.*}}@strchr{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strchr{{.*}} #2
 
 char *t9(void) { return strrchr("hello, world", 'w'); }
 // CHECK-LABEL: t9
-// CHECK: call{{.*}}@strrchr{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strrchr{{.*}} #2
 
 int t10(void) { return strcmp("foo", "bar"); }
 // CHECK-LABEL: t10
-// CHECK: call{{.*}}@strcmp{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strcmp{{.*}} #2
 
 int t11(void) { return strncmp("foo", "bar", 3); }
 // CHECK-LABEL: t11
-// CHECK: call{{.*}}@strncmp{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strncmp{{.*}} #2
 
 char *t12(char *x) { return strcpy(x, "foo"); }
 // CHECK-LABEL: t12
-// CHECK: call{{.*}}@strcpy{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strcpy{{.*}} #2
 
 char *t13(char *x) { return stpcpy(x, "foo"); }
 // CHECK-LABEL: t13
-// CHECK: call{{.*}}@stpcpy{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@stpcpy{{.*}} #2
 
 char *t14(char *x) { return strncpy(x, "foo", 3); }
 // CHECK-LABEL: t14
-// CHECK: call{{.*}}@strncpy{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strncpy{{.*}} #2
 
 size_t t15(void) { return strlen("foo"); }
 // CHECK-LABEL: t15
-// CHECK: call{{.*}}@strlen{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strlen{{.*}} #2
 
 char *t16(char *x) { return strpbrk(x, ""); }
 // CHECK-LABEL: t16
-// CHECK: call{{.*}}@strpbrk{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strpbrk{{.*}} #2
 
 size_t t17(char *x) { return strspn(x, ""); }
 // CHECK-LABEL: t17
-// CHECK: call{{.*}}@strspn{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strspn{{.*}} #2
 
 double t18(char **x) { return strtod("123.4", x); }
 // CHECK-LABEL: t18
-// CHECK: call{{.*}}@strtod{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strtod{{.*}} #2
 
 float t19(char **x) { return strtof("123.4", x); }
 // CHECK-LABEL: t19
-// CHECK: call{{.*}}@strtof{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strtof{{.*}} #2
 
 long double t20(char **x) { return strtold("123.4", x); }
 // CHECK-LABEL: t20
-// CHECK: call{{.*}}@strtold{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strtold{{.*}} #2
 
 long int t21(char **x) { return strtol("1234", x, 10); }
 // CHECK-LABEL: t21
-// CHECK: call{{.*}}@strtol{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strtol{{.*}} #2
 
 long int t22(char **x) { return strtoll("1234", x, 10); }
 // CHECK-LABEL: t22
-// CHECK: call{{.*}}@strtoll{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strtoll{{.*}} #2
 
 long int t23(char **x) { return strtoul("1234", x, 10); }
 // CHECK-LABEL: t23
-// CHECK: call{{.*}}@strtoul{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strtoul{{.*}} #2
 
 long int t24(char **x) { return strtoull("1234", x, 10); }
 // CHECK-LABEL: t24
-// CHECK: call{{.*}}@strtoull{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@strtoull{{.*}} #2
 
 void t25(FILE *fp, int *buf) {
   size_t x = fwrite(buf, sizeof(int), 10, fp);
   size_t y = fread(buf, sizeof(int), 10, fp);
 }
 // CHECK-LABEL: t25
-// CHECK: call{{.*}}@fwrite{{.*}} [[ATTR]]
-// CHECK: call{{.*}}@fread{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@fwrite{{.*}} #2
+// CHECK: call{{.*}}@fread{{.*}} #2
 
 FILE *t26(const char *path, const char *mode) {
   return fopen(path, mode);
 }
 // CHECK-LABEL: t26
-// CHECK: call{{.*}}@fopen{{.*}} [[ATTR]]
+// CHECK: call{{.*}}@fopen{{.*}} #2
 
-// CHECK: [[ATTR]] = { nobuiltin }
+// GLOBAL: #2 = { nobuiltin "no-builtins" }
+// INDIVIDUAL: #2 = { nobuiltin "no-builtin-ceil" "no-builtin-copysign" "no-builtin-cos" "no-builtin-fabs" "no-builtin-floor" "no-builtin-fopen" "no-builtin-fread" "no-builtin-fwrite" "no-builtin-stpcpy" "no-builtin-strcat" "no-builtin-strchr" "no-builtin-strcmp" "no-builtin-strcpy" "no-builtin-strlen" "no-builtin-strncat" "no-builtin-strncmp" "no-builtin-strncpy" "no-builtin-strpbrk" "no-builtin-strrchr" "no-builtin-strspn" "no-builtin-strtod" "no-builtin-strtof" "no-builtin-strtol" "no-builtin-strtold" "no-builtin-strtoll" "no-builtin-strtoul" "no-builtin-strtoull" }

clang/test/CodeGen/memccpy-libcall.c

 // RUN: %clang_cc1 -fno-builtin-memccpy -emit-llvm < %s| FileCheck %s
 
 typedef __SIZE_TYPE__ size_t;
 
 void *memccpy(void *, void const *, int, size_t);
 
 void test13(char *d, char *s, int c, size_t n) {
   // CHECK: call i8* @memccpy{{.*}} #2
   memccpy(d, s, c, n);
 }
 
-// CHECK: attributes #2 = { nobuiltin }
+// CHECK: attributes #2 = { nobuiltin "no-builtin-memccpy" }