[spirv] Allow aliasing builtin variables

This behavior is supported by FXC. Fixes microsoft#1690
ehsannas · Dec 13, 2018 · 4581b70 · 4581b70
1 parent c52113b
commit 4581b70
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 3 deletions.
diff --git a/tools/clang/include/clang/SPIRV/SpirvBuilder.h b/tools/clang/include/clang/SPIRV/SpirvBuilder.h
@@ -594,6 +594,17 @@ class SpirvBuilder {
 
   llvm::SetVector<spv::Capability> existingCapabilities;
   llvm::SetVector<Extension> existingExtensions;
+
+  /// A struct containing information regarding a builtin variable.
+  struct BuiltInVarInfo {
+    BuiltInVarInfo(spv::StorageClass s, spv::BuiltIn b, SpirvVariable *v)
+        : sc(s), builtIn(b), variable(v) {}
+    spv::StorageClass sc;
+    spv::BuiltIn builtIn;
+    SpirvVariable *variable;
+  };
+  /// Used as caches for all created builtin variables to avoid duplication.
+  llvm::SmallVector<BuiltInVarInfo, 16> builtinVars;
 };
 
 void SpirvBuilder::requireCapability(spv::Capability cap, SourceLocation loc) {

diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp
@@ -1013,8 +1013,14 @@ std::vector<SpirvVariable *> DeclResultIdMapper::collectStageVars() const {
   for (auto var : glPerVertex.getStageOutVars())
     vars.push_back(var);
 
-  for (const auto &var : stageVars)
-    vars.push_back(var.getSpirvInstr());
+  llvm::DenseSet<SpirvInstruction *> seenVars;
+  for (const auto &var : stageVars) {
+    auto *instr = var.getSpirvInstr();
+    if (seenVars.count(instr) == 0) {
+      vars.push_back(instr);
+      seenVars.insert(instr);
+    }
+  }
 
   return vars;
 }
@@ -1115,7 +1121,7 @@ bool DeclResultIdMapper::checkSemanticDuplication(bool forInput) {
     }
 
     // Allow builtin variables to alias each other. We already have uniqify
-    // mechanism in ModuleBuilder.
+    // mechanism in SpirvBuilder.
     if (var.isSpirvBuitin())
       continue;
 

diff --git a/tools/clang/lib/SPIRV/SpirvBuilder.cpp b/tools/clang/lib/SPIRV/SpirvBuilder.cpp
@@ -851,6 +851,17 @@ SpirvVariable *SpirvBuilder::addStageBuiltinVar(const SpirvType *type,
                                                 spv::StorageClass storageClass,
                                                 spv::BuiltIn builtin,
                                                 SourceLocation loc) {
+  // If the built-in variable has already been added (via a built-in alias),
+  // return the existing variable.
+  auto found = std::find_if(
+      builtinVars.begin(), builtinVars.end(),
+      [storageClass, builtin](const BuiltInVarInfo &varInfo) {
+        return varInfo.sc == storageClass && varInfo.builtIn == builtin;
+      });
+  if (found != builtinVars.end()) {
+    return found->variable;
+  }
+
   // Note: We store the underlying type in the variable, *not* the pointer type.
   auto *var =
       new (context) SpirvVariable(/*QualType*/ {}, /*id*/ 0, loc, storageClass);
@@ -862,13 +873,27 @@ SpirvVariable *SpirvBuilder::addStageBuiltinVar(const SpirvType *type,
       loc, var, spv::Decoration::BuiltIn, {static_cast<uint32_t>(builtin)});
   module->addDecoration(decor);
 
+  // Add variable to cache.
+  builtinVars.emplace_back(storageClass, builtin, var);
+
   return var;
 }
 
 SpirvVariable *SpirvBuilder::addStageBuiltinVar(QualType type,
                                                 spv::StorageClass storageClass,
                                                 spv::BuiltIn builtin,
                                                 SourceLocation loc) {
+  // If the built-in variable has already been added (via a built-in alias),
+  // return the existing variable.
+  auto found = std::find_if(
+      builtinVars.begin(), builtinVars.end(),
+      [storageClass, builtin](const BuiltInVarInfo &varInfo) {
+        return varInfo.sc == storageClass && varInfo.builtIn == builtin;
+      });
+  if (found != builtinVars.end()) {
+    return found->variable;
+  }
+
   // Note: We store the underlying type in the variable, *not* the pointer type.
   auto *var = new (context) SpirvVariable(type, /*id*/ 0, loc, storageClass);
   module->addVariable(var);
@@ -878,6 +903,9 @@ SpirvVariable *SpirvBuilder::addStageBuiltinVar(QualType type,
       loc, var, spv::Decoration::BuiltIn, {static_cast<uint32_t>(builtin)});
   module->addDecoration(decor);
 
+  // Add variable to cache.
+  builtinVars.emplace_back(storageClass, builtin, var);
+
   return var;
 }