[clang] Add per-global code model attribute #72078

heiher · 2023-11-13T02:48:13Z

This patch adds a per-global code model attribute, which can override the target's code model to access global variables.

Currently, the code model attribute is only supported on LoongArch. This patch also maps GCC's code model names to LLVM's, which allows for better compatibility between the two compilers.

Suggested-by: Arthur Eubanks aeubanks@google.com
Link: https://discourse.llvm.org/t/how-to-best-implement-code-model-overriding-for-certain-values/71816
Link: https://discourse.llvm.org/t/rfc-add-per-global-code-model-attribute/74944

llvmbot · 2023-11-13T02:48:44Z

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: hev (heiher)

Changes

This adds a per-global code model attribute, which can override the target's code model to access global variables.

Link: https://discourse.llvm.org/t/how-to-best-implement-code-model-overriding-for-certain-values/71816

Suggested-by: Arthur Eubanks <aeubanks@google.com>

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

8 Files Affected:

(modified) clang/include/clang/Basic/Attr.td (+8)
(modified) clang/include/clang/Basic/AttrDocs.td (+9)
(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
(modified) clang/lib/CodeGen/CodeGenModule.cpp (+13)
(modified) clang/lib/Sema/SemaDeclAttr.cpp (+20)
(modified) clang/test/CodeGen/attributes.c (+15)
(modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test (+1)
(added) clang/test/Sema/attr-model.c (+12)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 31434565becaec67..0d3742080435142f 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2694,6 +2694,14 @@ def PragmaClangTextSection : InheritableAttr {
   let Documentation = [InternalOnly];
 }
 
+def CodeModel : InheritableAttr {
+  let Spellings = [GCC<"model">];
+  let Args = [StringArgument<"Model">];
+  let Subjects =
+      SubjectList<[ GlobalVar ], ErrorDiag>;
+  let Documentation = [CodeModelDocs];
+}
+
 def Sentinel : InheritableAttr {
   let Spellings = [GCC<"sentinel">];
   let Args = [DefaultIntArgument<"Sentinel", 0>,
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index fa6f6acd0c30e883..ca09e4acb113003b 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -57,6 +57,15 @@ global variable or function should be in after translation.
   let Heading = "section, __declspec(allocate)";
 }
 
+def CodeModelDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+The ``model`` attribute allows you to specify a specific code model a
+global variable should be in after translation.
+  }];
+  let Heading = "model";
+}
+
 def UsedDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4614324babb1c917..f9305e21bd6a39b3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3401,6 +3401,9 @@ def warn_objc_redundant_literal_use : Warning<
 def err_attr_tlsmodel_arg : Error<"tls_model must be \"global-dynamic\", "
   "\"local-dynamic\", \"initial-exec\" or \"local-exec\"">;
 
+def err_attr_codemodel_arg : Error<"code_model must be \"tiny\", "
+  "\"small\", \"kernel\", \"medium\" or \"large\"">;
+
 def err_aix_attr_unsupported_tls_model : Error<"TLS model '%0' is not yet supported on AIX">;
 
 def err_tls_var_aligned_over_maximum : Error<
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 4c7f516e308ca003..f783130e3cbfd12e 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4774,6 +4774,19 @@ CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty,
         isExternallyVisible(D->getLinkageAndVisibility().getLinkage()))
       GV->setSection(".cp.rodata");
 
+    // Handle code model attribute
+    if (D->hasAttr<CodeModelAttr>()) {
+      if (const CodeModelAttr *CMA = D->getAttr<CodeModelAttr>()) {
+        auto CM = llvm::StringSwitch<llvm::CodeModel::Model>(CMA->getModel())
+          .Case("tiny", llvm::CodeModel::Tiny)
+          .Case("kernel", llvm::CodeModel::Kernel)
+          .Case("medium", llvm::CodeModel::Medium)
+          .Case("large", llvm::CodeModel::Large)
+          .Default(llvm::CodeModel::Small);
+        GV->setCodeModel(CM);
+      }
+    }
+
     // Check if we a have a const declaration with an initializer, we may be
     // able to emit it as available_externally to expose it's value to the
     // optimizer.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index cdb769a883550d0f..8646c494602a3ea4 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3369,6 +3369,23 @@ static void handleSectionAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   }
 }
 
+static void handleCodeModelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  StringRef Model;
+  SourceLocation LiteralLoc;
+  // Check that it is a string.
+  if (!S.checkStringLiteralArgumentAttr(AL, 0, Model, &LiteralLoc))
+    return;
+
+  // Check that the value.
+  if (Model != "tiny" && Model != "small"
+      && Model != "kernel" && Model != "medium" && Model != "large") {
+    S.Diag(LiteralLoc, diag::err_attr_codemodel_arg);
+    return;
+  }
+
+  D->addAttr(::new (S.Context) CodeModelAttr(S.Context, AL, Model));
+}
+
 // This is used for `__declspec(code_seg("segname"))` on a decl.
 // `#pragma code_seg("segname")` uses checkSectionName() instead.
 static bool checkCodeSegName(Sema &S, SourceLocation LiteralLoc,
@@ -9274,6 +9291,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   case ParsedAttr::AT_Section:
     handleSectionAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_CodeModel:
+    handleCodeModelAttr(S, D, AL);
+    break;
   case ParsedAttr::AT_RandomizeLayout:
     handleRandomizeLayoutAttr(S, D, AL);
     break;
diff --git a/clang/test/CodeGen/attributes.c b/clang/test/CodeGen/attributes.c
index 5afef72b747afb11..7783d87b84c29c9b 100644
--- a/clang/test/CodeGen/attributes.c
+++ b/clang/test/CodeGen/attributes.c
@@ -25,6 +25,21 @@ int t6 __attribute__((visibility("protected")));
 // CHECK: @t12 ={{.*}} global i32 0, section "SECT"
 int t12 __attribute__((section("SECT")));
 
+// CHECK: @tiny ={{.*}} global i32 0, code_model "tiny"
+int tiny __attribute__((model("tiny")));
+
+// CHECK: @small ={{.*}} global i32 0, code_model "small"
+int small __attribute__((model("small")));
+
+// CHECK: @kernel ={{.*}} global i32 0, code_model "kernel"
+int kernel __attribute__((model("kernel")));
+
+// CHECK: @medium ={{.*}} global i32 0, code_model "medium"
+int medium __attribute__((model("medium")));
+
+// CHECK: @large ={{.*}} global i32 0, code_model "large"
+int large __attribute__((model("large")));
+
 // CHECK: @t9 = weak{{.*}} alias void (...), ptr @__t8
 void __t8() {}
 void t9() __attribute__((weak, alias("__t8")));
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index 969794073a5f2e80..7a0b4f20cb88a5c7 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -48,6 +48,7 @@
 // CHECK-NEXT: CarriesDependency (SubjectMatchRule_variable_is_parameter, SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: Cleanup (SubjectMatchRule_variable_is_local)
 // CHECK-NEXT: CmseNSEntry (SubjectMatchRule_function)
+// CHECK-NEXT: CodeModel (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Cold (SubjectMatchRule_function)
 // CHECK-NEXT: Common (SubjectMatchRule_variable)
 // CHECK-NEXT: ConstInit (SubjectMatchRule_variable_is_global)
diff --git a/clang/test/Sema/attr-model.c b/clang/test/Sema/attr-model.c
new file mode 100644
index 0000000000000000..90b9713f5f32cd26
--- /dev/null
+++ b/clang/test/Sema/attr-model.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64 -verify -fsyntax-only %s
+
+#if !__has_attribute(model)
+#error "Should support model attribute"
+#endif
+
+int a __attribute((model("tiny"))); // no-warning
+int b __attribute((model("small"))); // no-warning
+int c __attribute((model("kernel"))); // no-warning
+int d __attribute((model("medium"))); // no-warning
+int e __attribute((model("large"))); // no-warning
+int f __attribute((model("normal"))); // expected-error {{code_model must be "tiny", "small", "kernel", "medium" or "large"}}

github-actions · 2023-11-13T02:51:02Z

✅ With the latest revision this PR passed the C/C++ code formatter.

heiher · 2023-11-13T02:51:20Z

Part 1/3: #72077
Part 3/3: #72079

heiher · 2023-11-13T03:04:34Z

cc @xen0n @xry111

clang/lib/Sema/SemaDeclAttr.cpp

This patch adds a per-global code model attribute, which can override the target's code model to access global variables. Currently, the code model attribute is only supported on LoongArch. This patch also maps GCC's code model names to LLVM's, which allows for better compatibility between the two compilers. Suggested-by: Arthur Eubanks <aeubanks@google.com> Signed-off-by: WANG Rui <wangrui@loongson.cn> Link: https://discourse.llvm.org/t/how-to-best-implement-code-model-overriding-for-certain-values/71816 Link: https://discourse.llvm.org/t/rfc-add-per-global-code-model-attribute/74944

xen0n

There should also be a test case that shows the attribute is not supported on a non-LoongArch target. Otherwise this mostly looks fine to me, thanks for pushing this forward!

clang/lib/Sema/SemaDeclAttr.cpp

clang/include/clang/Basic/AttrDocs.td

…ongArch target

rnk · 2023-12-05T17:33:59Z

Do folks feel like the attribute name is sufficiently descriptive? i.e. should it be __attribute__((code_model("asdf")))? Are we aiming for GCC compat here? What guides the naming choice?

xry111 · 2023-12-05T17:37:40Z

Do folks feel like the attribute name is sufficiently descriptive? i.e. should it be __attribute__((code_model("asdf")))? Are we aiming for GCC compat here? What guides the naming choice?

Yes, for GCC compat, so we don't need to add more #ifdef's in kernel.

As the author of the GCC attribute: the name choice was "borrowed" from IA64. I have to admit that maybe I learnt from a bad example (anyway IA64 is in a zombie state now since it's removed from the kernel), but for now I don't see too much benefit to rename this.

aeubanks · 2023-12-05T17:45:03Z

is it too late to change the gcc attribute name?

xry111 · 2023-12-05T17:46:40Z

is it too late to change the gcc attribute name?

It has been released in GCC 13, and GCC 14 is in stage 3 so a change must be deferred into GCC 15. And the kernel code already relies on it. So a change will render all previous kernel releases (and some future kernel releases) impossible to build with GCC 15+.

clang/test/CodeGen/LoongArch/attributes.cpp

clang/lib/CodeGen/CodeGenModule.cpp

clang/include/clang/Basic/DiagnosticSemaKinds.td

MaskRay · 2023-12-13T07:45:40Z

Add @erichkeane for the attribute change.

xen0n

Overall this seems fine, thanks!

But I'm not sure if reporting a more generic "the model attribute is not supported on this target" for non-LoongArch would be better: it doesn't give the false impression that the target doesn't support the specified code model.

clang/include/clang/Basic/DiagnosticSemaKinds.td

heiher · 2023-12-13T08:59:05Z

Overall this seems fine, thanks!

But I'm not sure if reporting a more generic "the model attribute is not supported on this target" for non-LoongArch would be better: it doesn't give the false impression that the target doesn't support the specified code model.

In my opinion I think this is better. I put it in a separate commit so we can easily revert it if someone thinks otherwise. thanks

heiher · 2024-01-04T10:46:28Z

@erichkeane gentle ping

clang/lib/Sema/SemaDeclAttr.cpp

This patch adds a per-global code model attribute, which can override the target's code model to access global variables. Currently, the code model attribute is only supported on LoongArch. This patch also maps GCC's code model names to LLVM's, which allows for better compatibility between the two compilers. Suggested-by: Arthur Eubanks <aeubanks@google.com> Link: https://discourse.llvm.org/t/how-to-best-implement-code-model-overriding-for-certain-values/71816 Link: https://discourse.llvm.org/t/rfc-add-per-global-code-model-attribute/74944 --------- Signed-off-by: WANG Rui <wangrui@loongson.cn>

heiher requested a review from aeubanks November 13, 2023 02:48

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Nov 13, 2023

heiher mentioned this pull request Nov 13, 2023

[llvm][IR] Add per-global code model attribute #72077

Merged

heiher marked this pull request as draft November 13, 2023 02:49

heiher mentioned this pull request Nov 13, 2023

[llvm][LoongArch] Support per-global code model attribute for LoongArch #72079

Merged

heiher force-pushed the model-attr-clang branch from 29c8916 to 323a8f8 Compare November 13, 2023 02:57

heiher requested review from nathanchance and SixWeining November 13, 2023 03:04

tschuett reviewed Nov 13, 2023

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

heiher force-pushed the model-attr-clang branch from 323a8f8 to 5333233 Compare December 5, 2023 02:00

heiher marked this pull request as ready for review December 5, 2023 02:03

heiher marked this pull request as draft December 5, 2023 02:14

heiher force-pushed the model-attr-clang branch from 5333233 to 20069f5 Compare December 5, 2023 03:50

heiher marked this pull request as ready for review December 5, 2023 03:50

heiher force-pushed the model-attr-clang branch from 20069f5 to 478cbb5 Compare December 5, 2023 04:11

heiher force-pushed the model-attr-clang branch from 478cbb5 to e386387 Compare December 5, 2023 07:25

xen0n reviewed Dec 5, 2023

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

clang/include/clang/Basic/AttrDocs.td Outdated Show resolved Hide resolved

heiher added 2 commits December 5, 2023 15:45

Address xen0n's comments

12b551c

Add a test case that shows the attribute is not supported on a non-Lo…

3a5cbbc

…ongArch target

heiher requested a review from xen0n December 11, 2023 14:35

MaskRay reviewed Dec 12, 2023

View reviewed changes

clang/test/CodeGen/LoongArch/attributes.cpp Outdated Show resolved Hide resolved

MaskRay reviewed Dec 12, 2023

View reviewed changes

clang/lib/CodeGen/CodeGenModule.cpp Outdated Show resolved Hide resolved

Address MaskRay's comments

157e5c8

heiher requested a review from MaskRay December 12, 2023 07:48

MaskRay reviewed Dec 13, 2023

View reviewed changes

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved

MaskRay requested review from erichkeane and removed request for xen0n December 13, 2023 07:45

MaskRay requested review from xen0n and MaskRay December 13, 2023 07:46

Address MaskRay's comments

27b000c

xen0n reviewed Dec 13, 2023

View reviewed changes

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved

heiher added 2 commits December 13, 2023 16:33

Address xen0n's comments

75cc3db

Address xen0n's comments

3609440

erichkeane reviewed Jan 4, 2024

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

Address erichkeane's comments

54dff5b

heiher requested a review from erichkeane January 5, 2024 07:26

erichkeane approved these changes Jan 5, 2024

View reviewed changes

heiher merged commit 4df5662 into llvm:main Jan 6, 2024
5 checks passed

heiher deleted the model-attr-clang branch January 6, 2024 00:52

heiher mentioned this pull request Jan 6, 2024

Tracking issue for CONFIG_MODULES on LoongArch ClangBuiltLinux/linux#1884

Closed

xen0n mentioned this pull request Jan 6, 2024

Content suggestion for This Week in LoongArch newsletter / 《每周一龙》新闻线索信箱 loongson-community/areweloongyet#16

Open

gbaraldi mentioned this pull request Feb 20, 2024

Default to the medium code model in x86 linux JuliaLang/julia#53391

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang] Add per-global code model attribute #72078

[clang] Add per-global code model attribute #72078

heiher commented Nov 13, 2023 •

edited

Loading

llvmbot commented Nov 13, 2023 •

edited

Loading

github-actions bot commented Nov 13, 2023 •

edited

Loading

heiher commented Nov 13, 2023 •

edited

Loading

heiher commented Nov 13, 2023

xen0n left a comment

rnk commented Dec 5, 2023

xry111 commented Dec 5, 2023

aeubanks commented Dec 5, 2023

xry111 commented Dec 5, 2023 •

edited

Loading

MaskRay commented Dec 13, 2023

xen0n left a comment

heiher commented Dec 13, 2023 •

edited

Loading

heiher commented Jan 4, 2024

[clang] Add per-global code model attribute #72078

[clang] Add per-global code model attribute #72078

Conversation

heiher commented Nov 13, 2023 • edited Loading

llvmbot commented Nov 13, 2023 • edited Loading

github-actions bot commented Nov 13, 2023 • edited Loading

heiher commented Nov 13, 2023 • edited Loading

heiher commented Nov 13, 2023

xen0n left a comment

Choose a reason for hiding this comment

rnk commented Dec 5, 2023

xry111 commented Dec 5, 2023

aeubanks commented Dec 5, 2023

xry111 commented Dec 5, 2023 • edited Loading

MaskRay commented Dec 13, 2023

xen0n left a comment

Choose a reason for hiding this comment

heiher commented Dec 13, 2023 • edited Loading

heiher commented Jan 4, 2024

heiher commented Nov 13, 2023 •

edited

Loading

llvmbot commented Nov 13, 2023 •

edited

Loading

github-actions bot commented Nov 13, 2023 •

edited

Loading

heiher commented Nov 13, 2023 •

edited

Loading

xry111 commented Dec 5, 2023 •

edited

Loading

heiher commented Dec 13, 2023 •

edited

Loading