[clang] dd2362a - [clang] Allow const variables with weak attribute to be overridden
Anders Waldenborg via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 3 14:46:28 PDT 2022
Author: Anders Waldenborg
Date: 2022-06-03T23:44:15+02:00
New Revision: dd2362a8bab312dffc42bfcb30ad1340840ef581
URL: https://github.com/llvm/llvm-project/commit/dd2362a8bab312dffc42bfcb30ad1340840ef581
DIFF: https://github.com/llvm/llvm-project/commit/dd2362a8bab312dffc42bfcb30ad1340840ef581.diff
LOG: [clang] Allow const variables with weak attribute to be overridden
A variable with `weak` attribute signifies that it can be replaced with
a "strong" symbol link time. Therefore it must not emitted with
"weak_odr" linkage, as that allows the backend to use its value in
optimizations.
The frontend already considers weak const variables as
non-constant (note_constexpr_var_init_weak diagnostic) so this change
makes frontend and backend consistent.
This commit reverses the
f49573d1 weak globals that are const should get weak_odr linkage.
commit from 2009-08-05 which introduced this behavior. Unfortunately
that commit doesn't provide any details on why the change was made.
This was discussed in
https://discourse.llvm.org/t/weak-attribute-semantics-on-const-variables/62311
Differential Revision: https://reviews.llvm.org/D126324
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/global-init.c
clang/test/CodeGen/weak_constant.c
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bcd7ec1b9316..c37773840147 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -336,6 +336,9 @@ Attribute Changes in Clang
builtins (corresponding to the specific names listed in the attribute) in the
body of the function the attribute is on.
+- When the ``weak`` attribute is applied to a const qualified variable clang no longer
+ tells the backend it is allowed to optimize based on initializer value.
+
Windows Support
---------------
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 464432350127..fe6295f3069c 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2959,7 +2959,7 @@ def WarnUnusedResult : InheritableAttr {
def Weak : InheritableAttr {
let Spellings = [GCC<"weak">];
let Subjects = SubjectList<[Var, Function, CXXRecord]>;
- let Documentation = [Undocumented];
+ let Documentation = [WeakDocs];
let SimpleHandler = 1;
}
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index aedf11a70753..872eb8a26673 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6477,3 +6477,69 @@ is only supported in compute shaders.
The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sv-groupindex
}];
}
+
+def WeakDocs : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+
+In supported output formats the ``weak`` attribute can be used to
+specify that a variable or function should be emitted as a symbol with
+``weak`` (if a definition) or ``extern_weak`` (if a declaration of an
+external symbol) `linkage
+<https://llvm.org/docs/LangRef.html#linkage-types>`_.
+
+If there is a non-weak definition of the symbol the linker will select
+that over the weak. They must have same type and alignment (variables
+must also have the same size), but may have a
diff erent value.
+
+If there are multiple weak definitions of same symbol, but no non-weak
+definition, they should have same type, size, alignment and value, the
+linker will select one of them (see also selectany_ attribute).
+
+If the ``weak`` attribute is applied to a ``const`` qualified variable
+definition that variable is no longer consider a compiletime constant
+as its value can change during linking (or dynamic linking). This
+means that it can e.g no longer be part of an initializer expression.
+
+.. code-block:: c
+
+ const int ANSWER __attribute__ ((weak)) = 42;
+
+ /* This function may be replaced link-time */
+ __attribute__ ((weak)) void debug_log(const char *msg)
+ {
+ fprintf(stderr, "DEBUG: %s\n", msg);
+ }
+
+ int main(int argc, const char **argv)
+ {
+ debug_log ("Starting up...");
+
+ /* This may print something else than "6 * 7 = 42",
+ if there is a non-weak definition of "ANSWER" in
+ an object linked in */
+ printf("6 * 7 = %d\n", ANSWER);
+
+ return 0;
+ }
+
+If an external declaration is marked weak and that symbol does not
+exist during linking (possibly dynamic) the address of the symbol will
+evaluate to NULL.
+
+.. code-block:: c
+
+ void may_not_exist(void) __attribute__ ((weak));
+
+ int main(int argc, const char **argv)
+ {
+ if (may_not_exist) {
+ may_not_exist();
+ } else {
+ printf("Function did not exist\n");
+ }
+ return 0;
+ }
+
+ }];
+}
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index b4ae266f8988..758887a3b627 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4899,12 +4899,8 @@ llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageForDeclarator(
if (Linkage == GVA_Internal)
return llvm::Function::InternalLinkage;
- if (D->hasAttr<WeakAttr>()) {
- if (IsConstantVariable)
- return llvm::GlobalVariable::WeakODRLinkage;
- else
- return llvm::GlobalVariable::WeakAnyLinkage;
- }
+ if (D->hasAttr<WeakAttr>())
+ return llvm::GlobalVariable::WeakAnyLinkage;
if (const auto *FD = D->getAsFunction())
if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
diff --git a/clang/test/CodeGen/global-init.c b/clang/test/CodeGen/global-init.c
index 11e214ade995..a69f02885611 100644
--- a/clang/test/CodeGen/global-init.c
+++ b/clang/test/CodeGen/global-init.c
@@ -9,14 +9,20 @@ int a = 242;
// This should get normal weak linkage.
int c __attribute__((weak))= 0;
-// CHECK: @c = weak{{.*}} global i32 0
+// CHECK: @c = weak global i32 0
-
-// Since this is marked const, it should get weak_odr linkage, since all
-// definitions have to be the same.
-// CHECK: @d = weak_odr constant i32 0
+// Even though is marked const, it should get still get "weak"
+// linkage, not "weak_odr" as the weak attribute makes it possible
+// that there is a strong definition that changes the value linktime,
+// so the value must not be considered constant.
+// CHECK: @d = weak constant i32 0
const int d __attribute__((weak))= 0;
+// However, "selectany" is similar to "weak", but isn't interposable
+// by a strong definition, and should appear as weak_odr.
+// CHECK: @e = weak_odr constant i32 17
+const int e __attribute__((selectany)) = 17;
+
// PR6168 "too many undefs"
struct ManyFields {
int a;
diff --git a/clang/test/CodeGen/weak_constant.c b/clang/test/CodeGen/weak_constant.c
index 726d2ef122e1..67bdd919684d 100644
--- a/clang/test/CodeGen/weak_constant.c
+++ b/clang/test/CodeGen/weak_constant.c
@@ -1,13 +1,31 @@
// RUN: %clang_cc1 -w -emit-llvm %s -O1 -o - | FileCheck %s
-// Check for bug compatibility with gcc.
+// This used to "check for bug compatibility with gcc".
+// Now it checks that that the "weak" declaration makes the value
+// fully interposable whereas a "selectany" one is handled as constant
+// and propagated.
+// CHECK: @x = weak {{.*}}constant i32 123
const int x __attribute((weak)) = 123;
+// CHECK: @y = weak_odr {{.*}}constant i32 234
+const int y __attribute((selectany)) = 234;
+
int* f(void) {
return &x;
}
int g(void) {
- // CHECK: ret i32 123
+ // CHECK: load i32, ptr @x
+ // CHECK-NOT: ret i32 123
return *f();
}
+
+int *k(void) {
+ return &y;
+}
+
+int l(void) {
+ // CHECK-NOT: load i32, ptr @y
+ // CHECK: ret i32 234
+ return *k();
+}
More information about the cfe-commits
mailing list