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

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet marked an inline comment as done.
gchatelet added inline comments.


================
Comment at: clang/test/CodeGen/libcalls-fno-builtin.c:163
 
-// 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" }
----------------
tejohnson wrote:
> 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?
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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71193/new/

https://reviews.llvm.org/D71193