Browse Source

Changed cbuffer-promoted global vars to have external linkage as to not have initializers (#1713)

Per LLVM rules, globals must be initialized unless they have external linkage. It makes sense to consider constant buffer values as having external linkage, since they are "linked in" by the application, so this change:
* Changes CBuffer globals to have external linkage and no initializer
* Fixes a few cases not handling the lack of initializers properly
* Fixes some tests which were testing for initialization
* Added a test for non-initialization of cbuffer-promoted globals
Tristan Labelle 6 years ago
parent
commit
d34f1dcd1b

+ 7 - 5
lib/HLSL/HLMatrixLowerPass.cpp

@@ -2326,11 +2326,13 @@ void HLMatrixLowerPass::runOnGlobalMatrixArray(GlobalVariable *GV) {
     Ty = ArrayType::get(Ty, *arraySize);
 
   Type *VecArrayTy = Ty;
-  Constant *OldInitVal = GV->getInitializer();
-  Constant *InitVal =
-      isa<UndefValue>(OldInitVal)
-          ? UndefValue::get(VecArrayTy)
-          : LowerMatrixArrayConst(OldInitVal, cast<ArrayType>(VecArrayTy));
+  Constant *InitVal = nullptr;
+  if (GV->hasInitializer()) {
+    Constant *OldInitVal = GV->getInitializer();
+    InitVal = isa<UndefValue>(OldInitVal)
+      ? UndefValue::get(VecArrayTy)
+      : LowerMatrixArrayConst(OldInitVal, cast<ArrayType>(VecArrayTy));
+  }
 
   bool isConst = GV->isConstant();
   GlobalVariable::ThreadLocalMode TLMode = GV->getThreadLocalMode();

+ 1 - 3
lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp

@@ -3558,9 +3558,7 @@ bool SROA_Helper::DoScalarReplacement(GlobalVariable *GV,
     return false;
 
   Module *M = GV->getParent();
-  Constant *Init = GV->getInitializer();
-  if (!Init)
-    Init = UndefValue::get(Ty);
+  Constant *Init = GV->hasInitializer() ? GV->getInitializer() : UndefValue::get(Ty);
   bool isConst = GV->isConstant();
 
   GlobalVariable::ThreadLocalMode TLMode = GV->getThreadLocalMode();

+ 3 - 0
tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -4770,6 +4770,9 @@ static GlobalVariable *CreateStaticGlobal(llvm::Module *M, GlobalVariable *GV) {
   GlobalVariable *NGV = cast<GlobalVariable>(GC);
   if (GV->hasInitializer()) {
     NGV->setInitializer(GV->getInitializer());
+  } else {
+    // The copy being static, it should be initialized per llvm rules
+    NGV->setInitializer(Constant::getNullValue(GV->getType()->getPointerElementType()));
   }
   // static global should have internal linkage
   NGV->setLinkage(GlobalValue::InternalLinkage);

+ 9 - 1
tools/clang/lib/CodeGen/CodeGenModule.cpp

@@ -2184,7 +2184,10 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D) {
   if (D->hasAttr<AnnotateAttr>())
     AddGlobalAnnotations(D, GV);
 
-  GV->setInitializer(Init);
+  // HLSL Change Begins.
+  if (!getLangOpts().HLSL || !D->isExternallyVisible())
+    GV->setInitializer(Init); // Resources and $Globals are not initialized
+  // HLSL Change Ends.
 
   // If it is safe to mark the global 'constant', do so now.
   GV->setConstant(!NeedsGlobalCtor && !NeedsGlobalDtor &&
@@ -2211,6 +2214,11 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D) {
       Context.getTargetInfo().getTriple().isMacOSX())
     Linkage = llvm::GlobalValue::InternalLinkage;
 
+  // HLSL Change Begins.
+  if (getLangOpts().HLSL && D->isExternallyVisible())
+    Linkage = llvm::GlobalValue::ExternalLinkage; //Resources and $Globals have no definition
+  // HLSL Change Ends.
+
   GV->setLinkage(Linkage);
   if (D->hasAttr<DLLImportAttr>())
     GV->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);

+ 68 - 0
tools/clang/test/CodeGenHLSL/quick-test/global-var-no-init.hlsl

@@ -0,0 +1,68 @@
+// RUN: %dxc -E main -T ps_6_0 > %s | FileCheck %s
+
+// CBuffer-promoted global variables should not have initializers
+// CHECK-NOT: {{.*var.*}} = constant float 0.000000e+00, align 4
+// CHECK-NOT: {{.*var_init.*}} = constant float 0.000000e+00, align 4
+// CHECK-NOT: {{.*const_var.*}} = constant float 0.000000e+00, align 4
+// CHECK-NOT: {{.*const_var_init.*}} = constant float 0.000000e+00, align 4
+// CHECK-NOT: {{.*extern_var.*}} = constant float 0.000000e+00, align 4
+// CHECK-NOT: {{.*extern_var_init.*}} = constant float 0.000000e+00, align 4
+// CHECK-NOT: {{.*extern_const_var.*}} = constant float 0.000000e+00, align 4
+// CHECK-NOT: {{.*extern_const_var_init.*}} = constant float 0.000000e+00, align 4
+
+// ... they should only exist in their CBuffer declaration
+// CHECK: cbuffer $Globals
+// CHECK: float var;
+// CHECK: float var_init;
+// CHECK: float const_var;
+// CHECK: float const_var_init;
+// CHECK: float extern_var;
+// CHECK: float extern_var_init;
+// CHECK: float extern_const_var;
+// CHECK: float extern_const_var_init;
+
+Texture2D tex;
+float var;
+float var_init = 1;
+const float const_var;
+const float const_var_init = 1;
+extern float extern_var;
+extern float extern_var_init = 1;
+extern const float extern_const_var;
+extern const float extern_const_var_init = 1;
+
+// Those get optimized away
+static float static_var;
+static float static_var_init = 1;
+static const float static_const_var;
+static const float static_const_var_init = 1;
+
+struct s
+{
+  // Those get optimized away
+  static float struct_static_var;
+  // static float struct_static_var_init = 1; // error: struct/class members cannot have default values
+  static const float struct_static_const_var;
+  static const float struct_static_const_var_init = 1;
+};
+
+float s::struct_static_var = 1;
+const float s::struct_static_const_var = 1;
+
+float main() : SV_Target {
+  static float func_static_var;
+  static float func_static_var_init = 1;
+  static const float func_static_const_var;
+  static const float func_static_const_var_init = 1;
+  return tex.Load((int3)0).x
+    + var + var_init
+    + const_var + const_var_init
+    + extern_var + extern_var_init
+    + extern_const_var + extern_const_var_init
+    + static_var + static_var_init
+    + static_const_var + static_const_var_init
+    + s::struct_static_var + /*s::struct_static_var_init*/
+    + s::struct_static_const_var + s::struct_static_const_var_init
+    + func_static_var + func_static_var_init
+    + func_static_const_var + func_static_const_var_init;
+}

+ 0 - 1
tools/clang/test/CodeGenHLSL/quick-test/global-var-write-test03.hlsl

@@ -1,6 +1,5 @@
 // RUN: %dxc -T ps_6_0 -E PSMain /Gec -HV 2016 > %s | FileCheck %s
 
-// CHECK: {{.*cbvar.*}} = constant float 0.000000e+00, align 4
 // CHECK: define void @PSMain()
 // CHECK: call %dx.types.CBufRet.f32 @dx.op.cbufferLoadLegacy.f32(i32 59, %dx.types.Handle %"$Globals_cbuffer", i32 0)
 // CHECK: %{{[a-z0-9]+.*[a-z0-9]*}} = fadd fast float %{{[a-z0-9]+.*[a-z0-9]*}}, 5.000000e+00

+ 8 - 8
tools/clang/test/CodeGenHLSL/quick-test/global-var-write-test04.hlsl

@@ -2,14 +2,14 @@
 
 // Writing to globals only supported with HV <= 2016
 
-// CHECK: {{.*g_s1.*}} = constant float 0.000000e+00, align 4
-// CHECK: {{.*g_s2.*}} = constant float 0.000000e+00, align 4
-// CHECK: {{.*g_v.*}} = constant <4 x float> zeroinitializer, align 4
-// CHECK: {{.*g_m1.*}} = constant %class.matrix.int.2.2 zeroinitializer, align 4
-// CHECK: {{.*g_m2.*}} = constant %class.matrix.int.2.2 zeroinitializer, align 4
-// CHECK: {{.*g_b.*}} = constant i32 0, align 1
-// CHECK: {{.*g_a.*}} = constant [5 x i32] zeroinitializer, align 4
-// CHECK: {{.*g_a2d.*}} = constant [3 x [2 x i32]] zeroinitializer, align 4
+// CHECK: {{.*g_s1.*}} = external constant float, align 4
+// CHECK: {{.*g_s2.*}} = external constant float, align 4
+// CHECK: {{.*g_v.*}} = external constant <4 x float>, align 4
+// CHECK: {{.*g_m1.*}} = external constant %class.matrix.int.2.2, align 4
+// CHECK: {{.*g_m2.*}} = external constant %class.matrix.int.2.2, align 4
+// CHECK: {{.*g_b.*}} = external constant i32, align 1
+// CHECK: {{.*g_a.*}} = external constant [5 x i32], align 4
+// CHECK: {{.*g_a2d.*}} = external constant [3 x [2 x i32]], align 4
 // CHECK-NOT: {{(.*g_s1.*)(.*static.copy.*)}} = internal global float 0.000000e+00
 // CHECK: {{(.*g_s2.*)(.*static.copy.*)}} = internal global float 0.000000e+00
 // CHECK-NOT: {{(.*g_v.*)(.*static.copy.*)}} = internal global <4 x float> zeroinitializer

+ 1 - 1
tools/clang/test/CodeGenHLSL/quick-test/incomp_array.hlsl

@@ -2,8 +2,8 @@
 
 // Verify no hang on incomplete array
 
-// CHECK: %struct.Special = type { <4 x float>, [3 x i32] }
 // CHECK: %"$Globals" = type { i32, %struct.Special }
+// CHECK: %struct.Special = type { <4 x float>, [3 x i32] }
 
 typedef const int inta[];