[libc] add support for function level attributes #79891

SchrodingerZhu · 2024-01-29T20:06:00Z

discussion at https://discourse.llvm.org/t/rfc-support-function-attributes-in-user-interface/76624

Demo macro:

#if !defined(__LIBC_CONST_ATTR) && defined(__cplusplus) && defined(__GNUC__)
#if __has_attribute(const)
#define __LIBC_CONST_ATTR [[gnu::const]]
#endif
#endif
#if !defined(__LIBC_CONST_ATTR) && defined(__GNUC__)
#if __has_attribute(const)
#define __LIBC_CONST_ATTR __attribute__((const))
#endif
#endif
#if !defined(__LIBC_CONST_ATTR)
#define __LIBC_CONST_ATTR
#endif

llvmbot · 2024-01-29T20:06:31Z

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/79891.diff

4 Files Affected:

(modified) libc/spec/spec.td (+21-1)
(modified) libc/spec/stdc.td (+6-1)
(modified) libc/utils/HdrGen/PublicAPICommand.cpp (+79)
(modified) libc/utils/HdrGen/PublicAPICommand.h (+2)

diff --git a/libc/spec/spec.td b/libc/spec/spec.td
index 818cfaee6b61c2b..9ed4fa4a31de678 100644
--- a/libc/spec/spec.td
+++ b/libc/spec/spec.td
@@ -169,10 +169,30 @@ class ArgSpec<Type type, list<Annotation> annotations = [], string name = ""> {
   string Name = name;
 }
 
-class FunctionSpec<string name, RetValSpec return, list<ArgSpec> args> {
+class FunctionAttr {}
+class GnuFunctionAttr<string attr> : FunctionAttr {
+  string Attr = attr;
+  string Style = "gnu";
+}
+class Cxx11FunctionAttr<string attr, string namespace> : FunctionAttr {
+  string Attr = attr;
+  string Namespace = namespace;
+  string Style = "cxx11";
+}
+class DeclspecFunctionAttr<string attr> : FunctionAttr {
+  string Attr = attr;
+  string Style = "declspec";
+}
+class FunctionAttrSpec<string macro, list<FunctionAttr> instances> {
+  list<FunctionAttr> Instances = instances;
+  string Macro = macro;
+}
+
+class FunctionSpec<string name, RetValSpec return, list<ArgSpec> args, list<FunctionAttrSpec> attrs = []> {
   string Name = name;
   RetValSpec Return = return;
   list<ArgSpec> Args = args;
+  list<FunctionAttrSpec> Attributes = attrs;
 }
 
 class ObjectSpec<string name, string type> {
diff --git a/libc/spec/stdc.td b/libc/spec/stdc.td
index 43b81c4abaa0e6b..a32a14812bec6fb 100644
--- a/libc/spec/stdc.td
+++ b/libc/spec/stdc.td
@@ -26,6 +26,11 @@ def StdC : StandardSpec<"stdc"> {
       []
   >;
 
+  FunctionAttrSpec ConstAttr = FunctionAttrSpec<"__LIBC_CONST_ATTR", [
+    Cxx11FunctionAttr<"const", "gnu">,
+    GnuFunctionAttr<"const">,
+  ]>;
+
   HeaderSpec CType = HeaderSpec<
       "ctype.h",
       [], // Macros
@@ -364,7 +369,7 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"ceilf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"ceill", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
 
-          FunctionSpec<"fabs", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
+          FunctionSpec<"fabs", RetValSpec<DoubleType>, [ArgSpec<DoubleType>], [ConstAttr]>,
           FunctionSpec<"fabsf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"fabsl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
           FunctionSpec<"fabsf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>]>,
diff --git a/libc/utils/HdrGen/PublicAPICommand.cpp b/libc/utils/HdrGen/PublicAPICommand.cpp
index b1c7a072658ffbd..d24637db0f7888f 100644
--- a/libc/utils/HdrGen/PublicAPICommand.cpp
+++ b/libc/utils/HdrGen/PublicAPICommand.cpp
@@ -15,6 +15,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/TableGen/Record.h"
+#include <algorithm>
+#include <llvm/ADT/STLExtras.h>
 
 // Text blocks for macro definitions and type decls can be indented to
 // suit the surrounding tablegen listing. We need to dedent such blocks
@@ -87,6 +89,71 @@ void writeAPIFromIndex(APIIndexer &G,
   if (G.Enumerations.size() != 0)
     OS << "};\n\n";
 
+  // declare macros for attributes
+  llvm::DenseMap<llvm::StringRef, llvm::Record *> MacroAttr;
+  for (auto &Name : EntrypointNameList) {
+    if (G.FunctionSpecMap.find(Name) == G.FunctionSpecMap.end()) {
+      continue;
+    }
+    llvm::Record *FunctionSpec = G.FunctionSpecMap[Name];
+    auto Attributes = FunctionSpec->getValueAsListOfDefs("Attributes");
+    for (auto *Attr : Attributes) {
+      MacroAttr[Attr->getValueAsString("Macro")] = Attr;
+    }
+  }
+
+  auto GetStyle = [](llvm::Record *Instance) {
+    auto Style = Instance->getValueAsString("Style");
+    if (Style == "cxx11")
+      return AttributeStyle::Cxx11;
+    if (Style == "gnu")
+      return AttributeStyle::Gnu;
+    return AttributeStyle::Declspec;
+  };
+
+  for (auto &[Macro, Attr] : MacroAttr) {
+    auto Instances = Attr->getValueAsListOfDefs("Instances");
+    llvm::SmallVector<std::pair<AttributeStyle, llvm::Record *>> Styles;
+    std::transform(Instances.begin(), Instances.end(),
+                   std::back_inserter(Styles),
+                   [&](llvm::Record *Instance)
+                       -> std::pair<AttributeStyle, llvm::Record *> {
+                     auto Style = GetStyle(Instance);
+                     return {Style, Instance};
+                   });
+    // Effectively sort on the first field
+    std::sort(Styles.begin(), Styles.end());
+    for (auto &[Style, Instance] : Styles) {
+      if (Style == AttributeStyle::Cxx11) {
+        OS << "#if !defined(" << Macro << ") && defined(__cplusplus)\n";
+        OS << "#define " << Macro << " [[";
+        auto Namespace = Instance->getValueAsString("Namespace");
+        if (Namespace != "")
+          OS << Namespace << "::";
+        OS << Instance->getValueAsString("Attr") << "]]\n";
+        OS << "#endif\n";
+      }
+      if (Style == AttributeStyle::Gnu) {
+        OS << "#if !defined(" << Macro << ") && defined(__GNUC__)\n";
+        OS << "#define " << Macro << " __attribute__((";
+        OS << Instance->getValueAsString("Attr") << "))\n";
+        OS << "#endif\n";
+      }
+      if (Style == AttributeStyle::Declspec) {
+        OS << "#if !defined(" << Macro << ") && defined(_MSC_VER)\n";
+        OS << "#define " << Macro << " __declspec(";
+        OS << Instance->getValueAsString("Attr") << ")\n";
+        OS << "#endif\n";
+      }
+    }
+    OS << "#if !defined(" << Macro << ")\n";
+    OS << "#define " << Macro << '\n';
+    OS << "#endif\n";
+  }
+
+  if (!MacroAttr.empty())
+    OS << '\n';
+
   OS << "__BEGIN_C_DECLS\n\n";
   for (auto &Name : EntrypointNameList) {
     if (G.FunctionSpecMap.find(Name) == G.FunctionSpecMap.end()) {
@@ -102,6 +169,14 @@ void writeAPIFromIndex(APIIndexer &G,
     llvm::Record *RetValSpec = FunctionSpec->getValueAsDef("Return");
     llvm::Record *ReturnType = RetValSpec->getValueAsDef("ReturnType");
 
+    auto Attributes = FunctionSpec->getValueAsListOfDefs("Attributes");
+    llvm::interleave(
+        Attributes.begin(), Attributes.end(),
+        [&](llvm::Record *Attr) { OS << Attr->getValueAsString("Macro"); },
+        [&]() { OS << ' '; });
+    if (!Attributes.empty())
+      OS << ' ';
+
     OS << G.getTypeAsString(ReturnType) << " " << Name << "(";
 
     auto ArgsList = FunctionSpec->getValueAsListOfDefs("Args");
@@ -124,6 +199,10 @@ void writeAPIFromIndex(APIIndexer &G,
     OS << "extern " << Type << " " << Name << ";\n";
   }
   OS << "__END_C_DECLS\n";
+
+  // undef the macros
+  for (auto &[Macro, Attr] : MacroAttr)
+    OS << "\n#undef " << Macro << '\n';
 }
 
 void writePublicAPI(llvm::raw_ostream &OS, llvm::RecordKeeper &Records) {}
diff --git a/libc/utils/HdrGen/PublicAPICommand.h b/libc/utils/HdrGen/PublicAPICommand.h
index fb0a7a81cf277a3..72401cfe068e33a 100644
--- a/libc/utils/HdrGen/PublicAPICommand.h
+++ b/libc/utils/HdrGen/PublicAPICommand.h
@@ -25,6 +25,8 @@ class RecordKeeper;
 
 namespace llvm_libc {
 
+enum class AttributeStyle { Cxx11 = 0, Gnu = 1, Declspec = 2 };
+
 class PublicAPICommand : public Command {
 private:
   const std::vector<std::string> &EntrypointNameList;

libc/utils/HdrGen/PublicAPICommand.cpp

nickdesaulniers · 2024-01-30T16:41:07Z

libc/utils/HdrGen/PublicAPICommand.cpp

+    }
+    OS << "#if !defined(" << Macro << ")\n";
+    OS << "#define " << Macro << '\n';
+    OS << "#endif\n";


I'd be curious to see examples of the resulting output.

I love function attributes, but using them portably is somewhat a PITA.

__has_attribute helps a lot, if your compiler supports it. If not, you have to rely on compiler version checks (or omit the attributes or explicitly not support those compiler versions).

So if we expose these on the client facing headers, we need to ensure that the result works with a wide range of compiler versions. Perhaps wider than we require to build llvm libc. I don't think we make the distinction in our docs, but we probably should.

We already require pretty modern GCC / Clang to compile. Worst case we can do defined(__has_attribute) && __has_attribute patterns where the default is nothing.

Gcc says:

In C code, if compiling for strict conformance to standards before C23, operand must be a valid identifier. Otherwise, operand may be optionally introduced by the attribute-scope:: prefix.

Clang seems not be able to parse __has_attribute(gnu::const).
As a result, we may not want to use __has_attribute as it is not applicable in many cases.

We already require pretty modern GCC / Clang to compile.

Right, but if we expose this headers contents to userspace, then that might be a different set of compilers to support.

Clang seems not be able to parse __has_attribute(gnu::const).

You don't use the gnu:: in the test.
https://godbolt.org/z/dP8Yj6PTM

I'd be curious to see examples of the resulting output.

Bump. Can you share how this changes the generated header declaration for fabs? That might be nice to include in the commit message even.

libc/spec/spec.td

libc/utils/HdrGen/PublicAPICommand.cpp

SchrodingerZhu · 2024-01-31T15:23:36Z

Example:

#if !defined(__LIBC_CONST_ATTR) && defined(__cplusplus) && defined(__GNUC__)
#define __LIBC_CONST_ATTR [[gnu::const]]
#endif
#if !defined(__LIBC_CONST_ATTR) && defined(__GNUC__)
#define __LIBC_CONST_ATTR __attribute__((const))
#endif
#if !defined(__LIBC_CONST_ATTR)
#define __LIBC_CONST_ATTR
#endif

libc/utils/HdrGen/PublicAPICommand.cpp

SchrodingerZhu · 2024-02-07T22:27:14Z

@nickdesaulniers A gentle ping for review since this is going to conflict with @lntue's change in #81010.

nickdesaulniers · 2024-02-09T17:30:28Z

sorry for the delays on review. Just got back from international travel.

SchrodingerZhu · 2024-02-10T17:44:31Z

sorry for the delays on review. Just got back from international travel.

No problem. Actually, my next week will be occupied by my school assignments :D. I will address the reviews above once I get spare time.

nickdesaulniers · 2024-02-12T16:53:31Z

libc/spec/spec.td

+class GnuFunctionAttr<string attr> : FunctionAttr<"gnu", attr> {}
+class Cxx11FunctionAttr<string attr, string namespace> : FunctionAttr<"cxx11", attr> {
+  // The namespace of the attribute, e.g. "gnu" or "clang".
+  string Namespace = namespace;


And I think the namespace can even be optional (i.e. for the standardized attributes such as noreturn and carries_dependency.

Actually, it is already optional (empty string is handled). I will add a note in comments.

nickdesaulniers · 2024-02-12T16:56:19Z

libc/utils/HdrGen/PublicAPICommand.cpp

+    }
+    OS << "#if !defined(" << Macro << ")\n";
+    OS << "#define " << Macro << '\n';
+    OS << "#endif\n";


I'd be curious to see examples of the resulting output.

Bump. Can you share how this changes the generated header declaration for fabs? That might be nice to include in the commit message even.

nickdesaulniers · 2024-02-12T21:00:38Z

Demo macro:

#if !defined(__LIBC_CONST_ATTR) && defined(__cplusplus) && defined(__GNUC__)
#if __has_attribute(const)
#define __LIBC_CONST_ATTR [[gnu::const]]
#endif
#endif
#if !defined(__LIBC_CONST_ATTR) && defined(__GNUC__)
#if __has_attribute(const)
#define __LIBC_CONST_ATTR __attribute__((const))
#endif
#endif
#if !defined(__LIBC_CONST_ATTR)
#define __LIBC_CONST_ATTR
#endif

Can you please change this to:

#ifndef __LIBC_CONST_ATTR
#if __has_attribute(const)
#ifdef __cplusplus
#define __LIBC_CONST_ATTR [[gnu::const]]
#else
#define __LIBC_CONST_ATTR __attribute__((const))
#endif // __cplusplus
#else
#define __LIBC_CONST_ATTR
#endif // __has_attribute(const)
#endif // __LIBC_CONST_ATTR

There's no point checking for __GNUC__ or !defined(__LIBC_CONST_ATTR) twice.

SchrodingerZhu · 2024-02-12T22:35:18Z

@nickdesaulniers I made it in this way so that codegen for each style is "context-independent". I can take a look to see if I can simplify the macro checks in context.

nickdesaulniers

ok, we can change this in the future when a need arises. For now, I'm happy to have this. thanks for the patch

SchrodingerZhu · 2024-02-15T17:20:37Z

Merge according to previous approval.

llvmbot added the libc label Jan 29, 2024

SchrodingerZhu commented Jan 29, 2024

View reviewed changes

libc/utils/HdrGen/PublicAPICommand.cpp Outdated Show resolved Hide resolved

SchrodingerZhu changed the title ~~Libc/attribute~~ [libc][RFC] add support for function level attributes Jan 29, 2024

SchrodingerZhu marked this pull request as draft January 29, 2024 20:24

SchrodingerZhu marked this pull request as ready for review January 30, 2024 14:39

nickdesaulniers reviewed Jan 30, 2024

View reviewed changes

SchrodingerZhu force-pushed the libc/attribute branch from bc16837 to 79419bd Compare January 31, 2024 14:27

SchrodingerZhu changed the title ~~[libc][RFC] add support for function level attributes~~ [libc] add support for function level attributes Jan 31, 2024

SchrodingerZhu requested a review from nickdesaulniers January 31, 2024 20:14

SchrodingerZhu commented Jan 31, 2024

View reviewed changes

libc/utils/HdrGen/PublicAPICommand.cpp Outdated Show resolved Hide resolved

nickdesaulniers mentioned this pull request Feb 5, 2024

[libc] abs is a const function #79650

Closed

SchrodingerZhu force-pushed the libc/attribute branch from d402341 to c3138d0 Compare February 5, 2024 14:32

SchrodingerZhu requested a review from jhuber6 February 5, 2024 15:03

[libc][RFC] add support for function level attributes

6c2ab9c

SchrodingerZhu force-pushed the libc/attribute branch from c3138d0 to 6c2ab9c Compare February 12, 2024 04:09

add attribute guard

03a5fbc

nickdesaulniers reviewed Feb 12, 2024

View reviewed changes

SchrodingerZhu added 4 commits February 12, 2024 13:17

adjust logic

2b37803

adjust comment

9ece53a

adjust comment indent

8824e6c

mention possible extension

f5cbf0e

SchrodingerZhu force-pushed the libc/attribute branch from 4589dde to f5cbf0e Compare February 12, 2024 18:31

SchrodingerZhu requested a review from nickdesaulniers February 12, 2024 20:52

nickdesaulniers approved these changes Feb 13, 2024

View reviewed changes

SchrodingerZhu merged commit 6cfbf8c into llvm:main Feb 15, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc] add support for function level attributes #79891

[libc] add support for function level attributes #79891

SchrodingerZhu commented Jan 29, 2024 •

edited

Loading

llvmbot commented Jan 29, 2024

nickdesaulniers Jan 30, 2024

jhuber6 Jan 30, 2024

SchrodingerZhu Jan 31, 2024 •

edited

Loading

nickdesaulniers Feb 9, 2024

nickdesaulniers Feb 12, 2024

SchrodingerZhu commented Jan 31, 2024

SchrodingerZhu commented Feb 7, 2024

nickdesaulniers commented Feb 9, 2024

SchrodingerZhu commented Feb 10, 2024

nickdesaulniers Feb 12, 2024

SchrodingerZhu Feb 12, 2024

nickdesaulniers Feb 12, 2024

nickdesaulniers commented Feb 12, 2024 •

edited

Loading

SchrodingerZhu commented Feb 12, 2024 •

edited

Loading

nickdesaulniers left a comment

SchrodingerZhu commented Feb 15, 2024

[libc] add support for function level attributes #79891

[libc] add support for function level attributes #79891

Conversation

SchrodingerZhu commented Jan 29, 2024 • edited Loading

llvmbot commented Jan 29, 2024

nickdesaulniers Jan 30, 2024

Choose a reason for hiding this comment

jhuber6 Jan 30, 2024

Choose a reason for hiding this comment

SchrodingerZhu Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

nickdesaulniers Feb 9, 2024

Choose a reason for hiding this comment

nickdesaulniers Feb 12, 2024

Choose a reason for hiding this comment

SchrodingerZhu commented Jan 31, 2024

SchrodingerZhu commented Feb 7, 2024

nickdesaulniers commented Feb 9, 2024

SchrodingerZhu commented Feb 10, 2024

nickdesaulniers Feb 12, 2024

Choose a reason for hiding this comment

SchrodingerZhu Feb 12, 2024

Choose a reason for hiding this comment

nickdesaulniers Feb 12, 2024

Choose a reason for hiding this comment

nickdesaulniers commented Feb 12, 2024 • edited Loading

SchrodingerZhu commented Feb 12, 2024 • edited Loading

nickdesaulniers left a comment

Choose a reason for hiding this comment

SchrodingerZhu commented Feb 15, 2024

SchrodingerZhu commented Jan 29, 2024 •

edited

Loading

SchrodingerZhu Jan 31, 2024 •

edited

Loading

nickdesaulniers commented Feb 12, 2024 •

edited

Loading

SchrodingerZhu commented Feb 12, 2024 •

edited

Loading