[spirv] Multiplex ModuleBuilder decorations (#1351)

ModuleBuilder had a catch-all function to apply decorations not
taking additional parameters, however, every single call to this
has an explicit decoration parameter. The switch only slows things
down and doesn't help with code length or anything else. The switch
is missing the other decorations that have custom functions, which
produce warnings. Rather than add a default: case, I've provided
custom decorate functions for each decoration.

tools/clang/include/clang/SPIRV/ModuleBuilder.h

@@ -404,9 +404,29 @@ public:
   void decorateHlslSemantic(uint32_t targetId, llvm::StringRef semantic,
                             llvm::Optional<uint32_t> memberIdx = llvm::None);
 
-  /// \brief Decorates the given target <result-id> with the given decoration
-  /// (without additional parameters).
-  void decorate(uint32_t targetId, spv::Decoration);
+  /// \brief Decorates the given target <result-id> with centroid
+  void decorateCentroid(uint32_t targetId);
+
+  /// \brief Decorates the given target <result-id> with flat
+  void decorateFlat(uint32_t targetId);
+
+  /// \brief Decorates the given target <result-id> with noperspective
+  void decorateNoPerspective(uint32_t targetId);
+
+  /// \brief Decorates the given target <result-id> with sample
+  void decorateSample(uint32_t targetId);
+
+  /// \brief Decorates the given target <result-id> with block
+  void decorateBlock(uint32_t targetId);
+
+  /// \brief Decorates the given target <result-id> with relaxedprecision
+  void decorateRelaxedPrecision(uint32_t targetId);
+
+  /// \brief Decorates the given target <result-id> with patch
+  void decoratePatch(uint32_t targetId);
+
+  /// \brief Decorates the given target <result-id> with nonuniformEXT
+  void decorateNonUniformEXT(uint32_t targetId);
 
   // === Type ===
 

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -1383,7 +1383,7 @@ bool DeclResultIdMapper::createStageVars(const hlsl::SigPoint *sigPoint,
     // TODO: the following may not be correct?
     if (sigPoint->GetSignatureKind() ==
         hlsl::DXIL::SignatureKind::PatchConstant)
-      theBuilder.decorate(varId, spv::Decoration::Patch);
+      theBuilder.decoratePatch(varId);
 
     // Decorate with interpolation modes for pixel shader input variables
     if (shaderModel.IsPS() && sigPoint->IsInput() &&
@@ -1801,20 +1801,20 @@ void DeclResultIdMapper::decoratePSInterpolationMode(const NamedDecl *decl,
                 "parameters in pixel shader",
                 decl->getLocation());
     } else {
-      theBuilder.decorate(varId, spv::Decoration::Flat);
+      theBuilder.decorateFlat(varId);
     }
   } else {
     // Do nothing for HLSLLinearAttr since its the default
     // Attributes can be used together. So cannot use else if.
     if (decl->getAttr<HLSLCentroidAttr>())
-      theBuilder.decorate(varId, spv::Decoration::Centroid);
+      theBuilder.decorateCentroid(varId);
     if (decl->getAttr<HLSLNoInterpolationAttr>())
-      theBuilder.decorate(varId, spv::Decoration::Flat);
+      theBuilder.decorateFlat(varId);
     if (decl->getAttr<HLSLNoPerspectiveAttr>())
-      theBuilder.decorate(varId, spv::Decoration::NoPerspective);
+      theBuilder.decorateNoPerspective(varId);
     if (decl->getAttr<HLSLSampleAttr>()) {
       theBuilder.requireCapability(spv::Capability::SampleRateShading);
-      theBuilder.decorate(varId, spv::Decoration::Sample);
+      theBuilder.decorateSample(varId);
     }
   }
 }

tools/clang/lib/SPIRV/ModuleBuilder.cpp

@@ -479,7 +479,7 @@ uint32_t ModuleBuilder::createImageSample(
   if (isNonUniform) {
     // The sampled image will be used to access resource's memory, so we need
     // to decorate it with NonUniformEXT.
-    decorate(sampledImgId, spv::Decoration::NonUniformEXT);
+    decorateNonUniformEXT(sampledImgId);
   }
 
   uint32_t texelId = theContext.takeNextId();
@@ -582,7 +582,7 @@ uint32_t ModuleBuilder::createImageGather(
   if (isNonUniform) {
     // The sampled image will be used to access resource's memory, so we need
     // to decorate it with NonUniformEXT.
-    decorate(sampledImgId, spv::Decoration::NonUniformEXT);
+    decorateNonUniformEXT(sampledImgId);
   }
 
   llvm::SmallVector<uint32_t, 2> params;
@@ -887,36 +887,43 @@ void ModuleBuilder::decorateSpecId(uint32_t targetId, uint32_t specId) {
   theModule.addDecoration(d, targetId);
 }
 
-void ModuleBuilder::decorate(uint32_t targetId, spv::Decoration decoration) {
-  const Decoration *d = nullptr;
-  switch (decoration) {
-  case spv::Decoration::Centroid:
-    d = Decoration::getCentroid(theContext);
-    break;
-  case spv::Decoration::Flat:
-    d = Decoration::getFlat(theContext);
-    break;
-  case spv::Decoration::NoPerspective:
-    d = Decoration::getNoPerspective(theContext);
-    break;
-  case spv::Decoration::Sample:
-    d = Decoration::getSample(theContext);
-    break;
-  case spv::Decoration::Block:
-    d = Decoration::getBlock(theContext);
-    break;
-  case spv::Decoration::RelaxedPrecision:
-    d = Decoration::getRelaxedPrecision(theContext);
-    break;
-  case spv::Decoration::Patch:
-    d = Decoration::getPatch(theContext);
-    break;
-  case spv::Decoration::NonUniformEXT:
-    d = Decoration::getNonUniformEXT(theContext);
-    break;
-  }
+void ModuleBuilder::decorateCentroid(uint32_t targetId) {
+  const Decoration *d = Decoration::getCentroid(theContext);
+  theModule.addDecoration(d, targetId);
+}
+
+void ModuleBuilder::decorateFlat(uint32_t targetId) {
+  const Decoration *d = Decoration::getFlat(theContext);
+  theModule.addDecoration(d, targetId);
+}
+
+void ModuleBuilder::decorateNoPerspective(uint32_t targetId) {
+  const Decoration *d = Decoration::getNoPerspective(theContext);
+  theModule.addDecoration(d, targetId);
+}
+
+void ModuleBuilder::decorateSample(uint32_t targetId) {
+  const Decoration *d = Decoration::getSample(theContext);
+  theModule.addDecoration(d, targetId);
+}
+
+void ModuleBuilder::decorateBlock(uint32_t targetId) {
+  const Decoration *d = Decoration::getBlock(theContext);
+  theModule.addDecoration(d, targetId);
+}
+
+void ModuleBuilder::decorateRelaxedPrecision(uint32_t targetId) {
+  const Decoration *d = Decoration::getRelaxedPrecision(theContext);
+  theModule.addDecoration(d, targetId);
+}
+
+void ModuleBuilder::decoratePatch(uint32_t targetId) {
+  const Decoration *d = Decoration::getPatch(theContext);
+  theModule.addDecoration(d, targetId);
+}
 
-  assert(d && "unimplemented decoration");
+void ModuleBuilder::decorateNonUniformEXT(uint32_t targetId) {
+  const Decoration *d = Decoration::getNonUniformEXT(theContext);
   theModule.addDecoration(d, targetId);
 }
 

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -899,7 +899,7 @@ SpirvEvalInfo SPIRVEmitter::loadIfGLValue(const Expr *expr,
   // Decorate with NonUniformEXT if loading from a pointer with that property.
   // We are likely loading an element from the resource array here.
   if (info.isNonUniform()) {
-    theBuilder.decorate(loadedId, spv::Decoration::NonUniformEXT);
+    theBuilder.decorateNonUniformEXT(loadedId);
   }
 
   // Special-case: According to the SPIR-V Spec: There is no physical size or
@@ -1350,7 +1350,7 @@ void SPIRVEmitter::doVarDecl(const VarDecl *decl) {
   }
 
   if (TypeTranslator::isRelaxedPrecisionType(decl->getType(), spirvOptions)) {
-    theBuilder.decorate(varId, spv::Decoration::RelaxedPrecision);
+    theBuilder.decorateRelaxedPrecision(varId);
   }
 
   // All variables that are of opaque struct types should request legalization.
@@ -2947,7 +2947,7 @@ SPIRVEmitter::processTextureLevelOfDetail(const CXXMemberCallExpr *expr) {
   if (objectInfo.isNonUniform() || samplerState.isNonUniform()) {
     // The sampled image will be used to access resource's memory, so we need
     // to decorate it with NonUniformEXT.
-    theBuilder.decorate(sampledImage, spv::Decoration::NonUniformEXT);
+    theBuilder.decorateNonUniformEXT(sampledImage);
   }
 
   // The result type of OpImageQueryLod must be a float2.
@@ -3131,7 +3131,7 @@ SpirvEvalInfo SPIRVEmitter::processBufferTextureLoad(
 
   if (objectInfo.isNonUniform()) {
     // Decoreate the image handle for OpImageFetch/OpImageRead
-    theBuilder.decorate(objectInfo, spv::Decoration::NonUniformEXT);
+    theBuilder.decorateNonUniformEXT(objectInfo);
   }
 
   // For Texture2DMS and Texture2DMSArray, Sample must be used rather than Lod.
@@ -5698,7 +5698,7 @@ SPIRVEmitter::tryToAssignToRWBufferRWTexture(const Expr *lhs,
     theBuilder.createImageWrite(imageType, imageId, locId, rhs);
     if (baseInfo.isNonUniform()) {
       // Decorate the image handle for OpImageWrite
-      theBuilder.decorate(imageId, spv::Decoration::NonUniformEXT);
+      theBuilder.decorateNonUniformEXT(imageId);
     }
     return rhs;
   }
@@ -6701,7 +6701,7 @@ SPIRVEmitter::processIntrinsicInterlockedMethod(const CallExpr *expr,
       if (baseId.isNonUniform()) {
         // Image texel pointer will used to access image memory. Vulkan requires
         // it to be decorated with NonUniformEXT.
-        theBuilder.decorate(ptr, spv::Decoration::NonUniformEXT);
+        theBuilder.decorateNonUniformEXT(ptr);
       }
     }
   }
@@ -6763,7 +6763,7 @@ SPIRVEmitter::processIntrinsicNonUniformResourceIndex(const CallExpr *expr) {
   // image instructions) and the resource descriptor being accessed is not
   // dynamically uniform, then the operand corresponding to that resource (e.g.
   // the pointer or sampled image operand) must be decorated with NonUniformEXT.
-  theBuilder.decorate(index, spv::Decoration::NonUniformEXT);
+  theBuilder.decorateNonUniformEXT(index);
 
   return index;
 }