CodeGen: Allow annotations on globals in non-zero address space

[nhaehnle]

Summary

Attribute annotations are recorded in a special global composite variable
that points to annotation strings and the annotated objects.

As a restriction of the LLVM IR type system, those pointers are all
pointers to address space 0, so let's insert an addrspacecast when the
annotated global is in a non-0 address space.

Since this addrspacecast is only reachable from the global annotations
object, this should allow us to represent annotations on all globals
regardless of which addrspacecasts are usually legal for the target.

[merge_guards_bot]

Build result: pass - 60628 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[rjmccall]

The questions I'd like to have answered before I can approve this are:

• whether there are clients of @llvm.global.annotations that will have problems with non-0 address spaces and
• whether this will interfere with someday having an invariant that addrspacecast is only used to do legal conversions.

[nhaehnle]

In D71208#1776202, @rjmccall wrote:

The questions I'd like to have answered before I can approve this are:

• whether there are clients of @llvm.global.annotations that will have problems with non-0 address spaces and

Yes, that's where this patch is ultimately coming from. I have some work-in-progress code to generate IRBuilder-using C++ code from LLVM IR, which we're currently using internally (with plans to upstream later next year) for an AMDGPU frontend, where non-0 address spaces are used. Using annotations is the least-invasive way I've found to attach required additional data to LLVM IR generated from C...

• whether this will interfere with someday having an invariant that addrspacecast is only used to do legal conversions.

That could potentially be a problem at some point, but this patch isn't making the situation any worse. Without this patch, Clang just unconditionally crashes (hits an assertion) when there's an annotation on a non-0 address space variable. With this patch, in the worst case the crash/assertion comes later...

[rjmccall]

In D71208#1776633, @nhaehnle wrote:

In D71208#1776202, @rjmccall wrote:

The questions I'd like to have answered before I can approve this are:

• whether there are clients of @llvm.global.annotations that will have problems with non-0 address spaces and

Yes, that's where this patch is ultimately coming from. I have some work-in-progress code to generate IRBuilder-using C++ code from LLVM IR, which we're currently using internally (with plans to upstream later next year) for an AMDGPU frontend, where non-0 address spaces are used. Using annotations is the least-invasive way I've found to attach required additional data to LLVM IR generated from C...

My concern is that there's something that's going to blow up or miscompile if we start passing in constants that aren't in a regular address space. Aren't there kinds of annotations which get persisted into the emitted code?

• whether this will interfere with someday having an invariant that addrspacecast is only used to do legal conversions.

That could potentially be a problem at some point, but this patch isn't making the situation any worse. Without this patch, Clang just unconditionally crashes (hits an assertion) when there's an annotation on a non-0 address space variable. With this patch, in the worst case the crash/assertion comes later...

Well, it's making it worse in the sense that this global-annotations list would become a blocker for tightening the representation.

[nhaehnle]

My concern is that there's something that's going to blow up or miscompile if we start passing in constants that aren't in a regular address space. Aren't there kinds of annotations which get persisted into the emitted code?

Annotations don't seem to be used for much at the moment in the first place. They're definitely not emitted in the resulting binary by default. Also, the only testcase in LLVM proper that has @llvm.global.annotations also happens to have addrspacecasts in there, so at least at some point in the past somebody thought it's okay to allow those casts there.

Also: what's the alternative?

[rjmccall]

In D71208#1776667, @nhaehnle wrote:

My concern is that there's something that's going to blow up or miscompile if we start passing in constants that aren't in a regular address space. Aren't there kinds of annotations which get persisted into the emitted code?

Annotations don't seem to be used for much at the moment in the first place. They're definitely not emitted in the resulting binary by default. Also, the only testcase in LLVM proper that has @llvm.global.annotations also happens to have addrspacecasts in there, so at least at some point in the past somebody thought it's okay to allow those casts there.

Alright, I relent. This looks okay to me.

clang/lib/CodeGen/CodeGenModule.cpp

 llvm::Constant *CodeGenModule::EmitAnnotateAttr(llvm::GlobalValue *GV,
                                                 const AnnotateAttr *AA,
                                                 SourceLocation L) {
   // Get the globals for file name, annotation, and the line number.
   llvm::Constant *AnnoGV = EmitAnnotationString(AA->getAnnotation()),
                  *UnitGV = EmitAnnotationUnit(L),
                  *LineNoCst = EmitAnnotationLineNo(L);
 
+  llvm::Constant *ASZeroGV = GV;
+  if (GV->getAddressSpace() != 0) {
+    ASZeroGV = llvm::ConstantExpr::getAddrSpaceCast(
+                   GV, GV->getValueType()->getPointerTo(0));
+  }
+
   // Create the ConstantStruct for the global annotation.
   llvm::Constant *Fields[4] = {
-    llvm::ConstantExpr::getBitCast(GV, Int8PtrTy),
+    llvm::ConstantExpr::getBitCast(ASZeroGV, Int8PtrTy),
     llvm::ConstantExpr::getBitCast(AnnoGV, Int8PtrTy),
     llvm::ConstantExpr::getBitCast(UnitGV, Int8PtrTy),
     LineNoCst
   };
   return llvm::ConstantStruct::getAnon(Fields);
 }
 
 void CodeGenModule::AddGlobalAnnotations(const ValueDecl *D,

clang/test/CodeGen/annotations-global.c

 // RUN: %clang_cc1 %s -emit-llvm -o %t1
 // RUN: FileCheck --check-prefix=FOO %s < %t1
 // RUN: FileCheck --check-prefix=A %s < %t1
 // RUN: FileCheck --check-prefix=BAR %s < %t1
 // RUN: FileCheck --check-prefix=FOOS %s < %t1
+// RUN: FileCheck --check-prefix=ADDRSPACE %s < %t1
 // END.
 
 static __attribute((annotate("sfoo_0"))) __attribute((annotate("sfoo_1"))) char sfoo;
 __attribute((annotate("foo_0"))) __attribute((annotate("foo_1"))) char foo;
 
 void __attribute((annotate("ann_a_0"))) __attribute((annotate("ann_a_1"))) __attribute((annotate("ann_a_2"))) __attribute((annotate("ann_a_3"))) a(char *a);
 void __attribute((annotate("ann_a_0"))) __attribute((annotate("ann_a_1"))) a(char *a) {
   __attribute__((annotate("bar_0"))) __attribute__((annotate("bar_1"))) static char bar;
   sfoo = 0;
 }
 
+__attribute((address_space(1))) __attribute__((annotate("addrspace1_ann"))) char addrspace1_var;
+
 // FOOS: target triple
 // FOOS: private unnamed_addr constant [7 x i8] c"sfoo_{{.}}\00", section "llvm.metadata"
 // FOOS: private unnamed_addr constant [7 x i8] c"sfoo_{{.}}\00", section "llvm.metadata"
 // FOOS-NOT: sfoo_
-// FOOS: @llvm.global.annotations = appending global [10 x { i8*, i8*, i8*, i32 }] {{.*}}i8* @sfoo{{.*}}i8* @sfoo{{.*}}, section "llvm.metadata"
+// FOOS: @llvm.global.annotations = appending global [11 x { i8*, i8*, i8*, i32 }] {{.*}}i8* @sfoo{{.*}}i8* @sfoo{{.*}}, section "llvm.metadata"
 
 // FOO: target triple
 // FOO: private unnamed_addr constant [6 x i8] c"foo_{{.}}\00", section "llvm.metadata"
 // FOO: private unnamed_addr constant [6 x i8] c"foo_{{.}}\00", section "llvm.metadata"
 // FOO-NOT: foo_
-// FOO: @llvm.global.annotations = appending global [10 x { i8*, i8*, i8*, i32 }] {{.*}}i8* @foo{{.*}}i8* @foo{{.*}}, section "llvm.metadata"
+// FOO: @llvm.global.annotations = appending global [11 x { i8*, i8*, i8*, i32 }] {{.*}}i8* @foo{{.*}}i8* @foo{{.*}}, section "llvm.metadata"
 
 // A: target triple
 // A: private unnamed_addr constant [8 x i8] c"ann_a_{{.}}\00", section "llvm.metadata"
 // A: private unnamed_addr constant [8 x i8] c"ann_a_{{.}}\00", section "llvm.metadata"
 // A: private unnamed_addr constant [8 x i8] c"ann_a_{{.}}\00", section "llvm.metadata"
 // A: private unnamed_addr constant [8 x i8] c"ann_a_{{.}}\00", section "llvm.metadata"
 // A-NOT: ann_a_
-// A: @llvm.global.annotations = appending global [10 x { i8*, i8*, i8*, i32 }] {{.*}}i8* bitcast (void (i8*)* @a to i8*){{.*}}i8* bitcast (void (i8*)* @a to i8*){{.*}}i8* bitcast (void (i8*)* @a to i8*){{.*}}i8* bitcast (void (i8*)* @a to i8*){{.*}}, section "llvm.metadata"
+// A: @llvm.global.annotations = appending global [11 x { i8*, i8*, i8*, i32 }] {{.*}}i8* bitcast (void (i8*)* @a to i8*){{.*}}i8* bitcast (void (i8*)* @a to i8*){{.*}}i8* bitcast (void (i8*)* @a to i8*){{.*}}i8* bitcast (void (i8*)* @a to i8*){{.*}}, section "llvm.metadata"
 
 // BAR: target triple
 // BAR: private unnamed_addr constant [6 x i8] c"bar_{{.}}\00", section "llvm.metadata"
 // BAR: private unnamed_addr constant [6 x i8] c"bar_{{.}}\00", section "llvm.metadata"
 // BAR-NOT: bar_
-// BAR: @llvm.global.annotations = appending global [10 x { i8*, i8*, i8*, i32 }] {{.*}}i8* @a.bar{{.*}}i8* @a.bar{{.*}}, section "llvm.metadata"
+// BAR: @llvm.global.annotations = appending global [11 x { i8*, i8*, i8*, i32 }] {{.*}}i8* @a.bar{{.*}}i8* @a.bar{{.*}}, section "llvm.metadata"
+
+// ADDRSPACE: target triple
+// ADDRSPACE: @llvm.global.annotations = appending global {{.*}} addrspacecast (i8 addrspace(1)* @addrspace1_var to i8*), {{.*}}