Skip to content

[clang][X86] Support __attribute__((model("small"/"large"))) #124834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Feb 15, 2025
Merged

[clang][X86] Support __attribute__((model("small"/"large"))) #124834

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7c40169
[clang][X86] Support __attribute__((model("small"/"large")))
aeubanks Jan 28, 2025
8aa6b3f
address comments
aeubanks Jan 29, 2025
2fdf2ee
add nvptx RUN line to test, refactor test
aeubanks Jan 29, 2025
1a32c31
add more flags to RUN line
aeubanks Jan 29, 2025
7ce06bd
allow and ignore attribute on nvptx compiles
aeubanks Feb 3, 2025
526e8c3
Merge remote-tracking branch 'upstream/main' into model
aeubanks Feb 3, 2025
6779705
ignore amdgpu/spirv
aeubanks Feb 4, 2025
2263db1
update AttrDocs.td
aeubanks Feb 5, 2025
bf8f7d4
more descriptive docs (at least for x86-64)
aeubanks Feb 5, 2025
d314c8e
warn if host triple doesn't support attribute
aeubanks Feb 12, 2025
1729029
llvm::Triple&
aeubanks Feb 12, 2025
c664880
one more &
aeubanks Feb 12, 2025
f3ecb7b
Merge remote-tracking branch 'upstream/main' into model
aeubanks Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ Removed Compiler Flags
Attribute Changes in Clang
--------------------------

- Clang now allows ``__attribute__((model("small")))`` and
``__attribute__((model("large")))`` on non-TLS globals in x86-64 compilations.
This forces the global to be considered small or large in regards to the
x86-64 code model, regardless of the code model specified for the compilation.

Improvements to Clang's diagnostics
-----------------------------------

Expand Down
20 changes: 16 additions & 4 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ class TargetArch<list<string> arches> : TargetSpec {
}
def TargetARM : TargetArch<["arm", "thumb", "armeb", "thumbeb"]>;
def TargetAArch64 : TargetArch<["aarch64", "aarch64_be", "aarch64_32"]>;
def TargetAMDGPU : TargetArch<["amdgcn", "r600"]>;
def TargetAnyArm : TargetArch<!listconcat(TargetARM.Arches, TargetAArch64.Arches)>;
def TargetAVR : TargetArch<["avr"]>;
def TargetBPF : TargetArch<["bpfel", "bpfeb"]>;
Expand All @@ -462,7 +463,9 @@ def TargetMSP430 : TargetArch<["msp430"]>;
def TargetM68k : TargetArch<["m68k"]>;
def TargetRISCV : TargetArch<["riscv32", "riscv64"]>;
def TargetX86 : TargetArch<["x86"]>;
def TargetX86_64 : TargetArch<["x86_64"]>;
def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
def TargetSPIRV : TargetArch<["spirv", "spirv32", "spirv64"]>;
def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
def TargetNVPTX : TargetArch<["nvptx", "nvptx64"]>;
def TargetWindows : TargetSpec {
Expand Down Expand Up @@ -3117,11 +3120,20 @@ def PragmaClangTextSection : InheritableAttr {
let Documentation = [InternalOnly];
}

def CodeModel : InheritableAttr, TargetSpecificAttr<TargetLoongArch> {
// The code model attribute only applies to LoongArch and x86-64, but for NVPTX
Copy link
Collaborator

@AaronBallman AaronBallman Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Should we update AttrDocs.td?
  2. Why is it reasonable to silently ignore the attribute? (That's generally not something we ever do because it becomes almost impossible to realize that your attribute isn't behaving the way you expect.) Please add that description to the patch summary.

Copy link
Contributor Author

@aeubanks aeubanks Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated AttrDocs.td.

updated description, hopefully that makes sense and the previous discussion makes sense

// compilations that share code with the host, we want to ignore the attribute
// rather than warn on it.
def CodeModel
: InheritableAttr,
TargetSpecificAttr<TargetArch<!listconcat(
TargetLoongArch.Arches, TargetX86_64.Arches, TargetNVPTX.Arches,
TargetAMDGPU.Arches, TargetSPIRV.Arches)>> {
let Spellings = [GCC<"model">];
let Args = [EnumArgument<"Model", "llvm::CodeModel::Model", /*is_string=*/1,
["normal", "medium", "extreme"], ["Small", "Medium", "Large"],
/*opt=*/0, /*fake=*/0, /*isExternalType=*/1, /*isCovered=*/0>];
let Args = [EnumArgument<
"Model", "llvm::CodeModel::Model",
/*is_string=*/1, ["small", "normal", "medium", "large", "extreme"],
["Small", "Small", "Medium", "Large", "Large"],
/*opt=*/0, /*fake=*/0, /*isExternalType=*/1, /*isCovered=*/0>];
let Subjects = SubjectList<[NonTLSGlobalVar], ErrorDiag>;
let Documentation = [CodeModelDocs];
}
Expand Down
22 changes: 21 additions & 1 deletion clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
#include "llvm/IR/DerivedTypes.h"
#include "llvm/MC/MCSectionMachO.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/Triple.h"
Expand Down Expand Up @@ -2949,15 +2950,34 @@ static void handleSectionAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
}
}

static bool isValidCodeModelAttr(Sema &S, StringRef Str) {
if (S.Context.getTargetInfo().getTriple().isLoongArch()) {
return Str == "normal" || Str == "medium" || Str == "extreme";
} else {
assert(S.Context.getTargetInfo().getTriple().getArch() ==
llvm::Triple::x86_64 &&
"only loongarch/x86-64 supported");
return Str == "small" || Str == "large";
}
}

static void handleCodeModelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
StringRef Str;
SourceLocation LiteralLoc;
// Check that it is a string.
if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &LiteralLoc))
return;

// Ignore the attribute for GPU device compiles since it only applies to host
// globals.
if (S.Context.getTargetInfo().getTriple().isNVPTX() ||
Copy link
Collaborator

@erichkeane erichkeane Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should diagnose if this doesn't apply to host and device (see getAuxTriple). The only valid reason to ignore this is if we see it is not in host, but IS in device (aux).

Copy link
Contributor Author

@aeubanks aeubanks Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll get a warning on the host side in the case the host target doesn't support the attribute and there's no reason to emit the warning again on the device side right?

Copy link
Collaborator

@erichkeane erichkeane Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps? Perhaps not. WE aren't necessarily the compiler on both sides, so we're better off making sure we diagnose it as soon as we know what is going on.

Copy link
Collaborator

@rnk rnk Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we? I mean, I'm sure nvidia and vendors everywhere do crazy stuff, but I would like to clarify what the support scope of the Clang project is. My understanding of all the CUDA functionality we've ever added to Clang was that it assumes we're using the Clang compiler on both sides, not the janky nvcc split pre-processing model.

Copy link
Collaborator

@erichkeane erichkeane Feb 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its not just NVIDIA CUDA. SYCL does it, and IIRC, the OMP offload supports arbitrary host compilers as well. Either way, we should be diagnosing "not supported" as early as we know anyway, which means during 'device' compilation (if it is first).

I don't know anything about Clang CUDA though (nor very much about NVIDIA CUDA for that matter).

Copy link
Contributor Author

@aeubanks aeubanks Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now we warn if both the current target and getAuxTriple() don't support the attribute. however, if we're a pure GPU compile with no getAuxTriple(), we silently ignore the attribute since we don't know what the host could be. does this work?

S.Context.getTargetInfo().getTriple().isAMDGPU() ||
S.Context.getTargetInfo().getTriple().isSPIRV())
return;

llvm::CodeModel::Model CM;
if (!CodeModelAttr::ConvertStrToModel(Str, CM)) {
if (!CodeModelAttr::ConvertStrToModel(Str, CM) ||
!isValidCodeModelAttr(S, Str)) {
S.Diag(LiteralLoc, diag::err_attr_codemodel_arg) << Str;
return;
}
Expand Down
27 changes: 27 additions & 0 deletions clang/test/CodeGen/X86/codemodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown %s -o - | FileCheck %s

// CHECK: @_ZL2v1 ={{.*}} global i32 0, code_model "small"
static int v1 __attribute__((model("small")));

void use1() {
v1 = 1;
}

// CHECK: @v2 ={{.*}} global float 0.000000e+00, code_model "large"
float v2 __attribute__((model("large")));

// CHECK: @_ZL2v3IiE ={{.*}} global i32 0, code_model "small"
template <typename T>
static T v3 __attribute__((model("small")));

void use2() {
v3<int> = 1;
}
struct S {
double d;
};

typedef void (*F)();

// CHECK: @v4 ={{.*}} global ptr null, code_model "large"
F v4 __attribute__((model("large")));
92 changes: 37 additions & 55 deletions clang/test/Sema/attr-model.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,64 +1,46 @@
// RUN: %clang_cc1 -triple aarch64 -verify=expected,aarch64 -fsyntax-only %s
// RUN: %clang_cc1 -triple loongarch64 -verify=expected,loongarch64 -fsyntax-only %s
// RUN: %clang_cc1 -triple mips64 -verify=expected,mips64 -fsyntax-only %s
// RUN: %clang_cc1 -triple powerpc64 -verify=expected,powerpc64 -fsyntax-only %s
// RUN: %clang_cc1 -triple riscv64 -verify=expected,riscv64 -fsyntax-only %s
// RUN: %clang_cc1 -triple x86_64 -verify=expected,x86_64 -fsyntax-only %s
// RUN: %clang_cc1 -triple aarch64 -verify=unsupported -fsyntax-only %s
// RUN: %clang_cc1 -triple loongarch64 -verify=loongarch64 -fsyntax-only %s
// RUN: %clang_cc1 -triple mips64 -verify=unsupported -fsyntax-only %s
// RUN: %clang_cc1 -triple powerpc64 -verify=unsupported -fsyntax-only %s
// RUN: %clang_cc1 -triple riscv64 -verify=unsupported -fsyntax-only %s
// RUN: %clang_cc1 -triple x86_64 -verify=x86_64 -fsyntax-only %s
// RUN: %clang_cc1 -triple nvptx64-unknown-cuda -fcuda-is-device -x cuda -verify=ignored -fsyntax-only %s
// RUN: %clang_cc1 -triple amdgcn -verify=ignored -fsyntax-only %s
// RUN: %clang_cc1 -triple r600 -verify=ignored -fsyntax-only %s
// RUN: %clang_cc1 -triple spirv-linux-vulkan-library -verify=ignored -fsyntax-only %s
// RUN: %clang_cc1 -triple spirv32-unknown-unknown -verify=ignored -fsyntax-only %s
// RUN: %clang_cc1 -triple spirv64-unknown-unknown -verify=ignored -fsyntax-only %s

#if defined(__loongarch__) && !__has_attribute(model)
#if (defined(__loongarch__) || defined(__x86_64__)) && !__has_attribute(model)
#error "Should support model attribute"
#endif

int a __attribute((model("tiny"))); // aarch64-warning {{unknown attribute 'model' ignored}} \
int a __attribute((model("tiny"))); // unsupported-warning {{unknown attribute 'model' ignored}} \
// loongarch64-error {{code model 'tiny' is not supported on this target}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
int b __attribute((model("small"))); // aarch64-warning {{unknown attribute 'model' ignored}} \
// loongarch64-error {{code model 'small' is not supported on this target}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
int c __attribute((model("normal"))); // aarch64-warning {{unknown attribute 'model' ignored}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
int d __attribute((model("kernel"))); // aarch64-warning {{unknown attribute 'model' ignored}} \
// x86_64-error {{code model 'tiny' is not supported on this target}}
int b __attribute((model("small"))); // unsupported-warning {{unknown attribute 'model' ignored}} \
// loongarch64-error {{code model 'small' is not supported on this target}}
int c __attribute((model("normal"))); // unsupported-warning {{unknown attribute 'model' ignored}} \
// x86_64-error {{code model 'normal' is not supported on this target}}
int d __attribute((model("kernel"))); // unsupported-warning {{unknown attribute 'model' ignored}} \
// loongarch64-error {{code model 'kernel' is not supported on this target}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
int e __attribute((model("medium"))); // aarch64-warning {{unknown attribute 'model' ignored}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
int f __attribute((model("large"))); // aarch64-warning {{unknown attribute 'model' ignored}} \
// loongarch64-error {{code model 'large' is not supported on this target}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
int g __attribute((model("extreme"))); // aarch64-warning {{unknown attribute 'model' ignored}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
// x86_64-error {{code model 'kernel' is not supported on this target}}
int e __attribute((model("medium"))); // unsupported-warning {{unknown attribute 'model' ignored}} \
// x86_64-error {{code model 'medium' is not supported on this target}}
int f __attribute((model("large"))); // unsupported-warning {{unknown attribute 'model' ignored}} \
// loongarch64-error {{code model 'large' is not supported on this target}}
int g __attribute((model("extreme"))); // unsupported-warning {{unknown attribute 'model' ignored}} \
// x86_64-error {{code model 'extreme' is not supported on this target}}

void __attribute((model("extreme"))) h() {} // aarch64-warning {{unknown attribute 'model' ignored}} \
void __attribute((model("extreme"))) h() {} // unsupported-warning {{unknown attribute 'model' ignored}} \
// ignored-error {{'model' attribute only applies to non-TLS global variables}} \
// loongarch64-error {{'model' attribute only applies to non-TLS global variables}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
// x86_64-error {{'model' attribute only applies to non-TLS global variables}}

thread_local int i __attribute((model("extreme"))); // aarch64-warning {{unknown attribute 'model' ignored}} \
// loongarch64-error {{'model' attribute only applies to non-TLS global variables}} \
// mips64-warning {{unknown attribute 'model' ignored}} \
// powerpc64-warning {{unknown attribute 'model' ignored}} \
// riscv64-warning {{unknown attribute 'model' ignored}} \
// x86_64-warning {{unknown attribute 'model' ignored}}
// NVPTX doesn't support thread_local at all.
#if !defined(__NVPTX__) && !defined(__AMDGCN__) && !defined(__R600__) && !defined(__SPIRV__)
thread_local
#endif
int i __attribute((model("extreme"))); // unsupported-warning {{unknown attribute 'model' ignored}} \
// loongarch64-error {{'model' attribute only applies to non-TLS global variables}} \
// x86_64-error {{'model' attribute only applies to non-TLS global variables}}