[spirv] Add support for -fvk-invert-y (#967)

This is to accommodate Vulkan's coordinate system, which is different
from DX's.

docs/SPIR-V.rst

@@ -2316,6 +2316,9 @@ codegen for Vulkan:
 - ``-fvk-ignore-unused-resources``: Avoids emitting SPIR-V code for resources
   defined but not statically referenced by the call tree of the entry point
   in question.
+- ``-fvk-invert-y``: Inverts SV_Position.y before writing to stage output.
+  Used to accommodate the difference between Vulkan's coordinate system and
+  DirectX's. Only allowed in VS/DS/GS.
 - ``-fvk-stage-io-order={alpha|decl}``: Assigns the stage input/output variable
   location number according to alphabetical order or declaration order. See
   `HLSL semantic and Vulkan Location`_ for more details.

include/dxc/Support/HLSLOptions.h

@@ -159,6 +159,7 @@ public:
 #ifdef ENABLE_SPIRV_CODEGEN
   bool GenSPIRV; // OPT_spirv
   bool VkIgnoreUnusedResources; // OPT_fvk_ignore_used_resources
+  bool VkInvertY; // OPT_fvk_invert_y
   llvm::StringRef VkStageIoOrder; // OPT_fvk_stage_io_order
   llvm::SmallVector<uint32_t, 4> VkBShift; // OPT_fvk_b_shift
   llvm::SmallVector<uint32_t, 4> VkTShift; // OPT_fvk_t_shift

include/dxc/Support/HLSLOptions.td

@@ -248,6 +248,8 @@ def fvk_s_shift : MultiArg<["-"], "fvk-s-shift", 2>, MetaVarName<"<shift> <space
   HelpText<"Specify Vulkan binding number shift for s-type register">;
 def fvk_u_shift : MultiArg<["-"], "fvk-u-shift", 2>, MetaVarName<"<shift> <space>">, Group<spirv_Group>, Flags<[CoreOption, DriverOption]>,
   HelpText<"Specify Vulkan binding number shift for u-type register">;
+def fvk_invert_y: Flag<["-"], "fvk-invert-y">, Group<spirv_Group>, Flags<[CoreOption, DriverOption]>,
+  HelpText<"Invert SV_Position.y in VS/DS/GS to accommodate Vulkan's coordinate system">;
 // SPIRV Change Ends
 
 //////////////////////////////////////////////////////////////////////////////

lib/DxcSupport/HLSLOptions.cpp

@@ -481,6 +481,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
   // SPIRV Change Starts
 #ifdef ENABLE_SPIRV_CODEGEN
   const bool genSpirv = opts.GenSPIRV = Args.hasFlag(OPT_spirv, OPT_INVALID, false);
+  opts.VkInvertY = Args.hasFlag(OPT_fvk_invert_y, OPT_INVALID, false);
   opts.VkIgnoreUnusedResources = Args.hasFlag(OPT_fvk_ignore_unused_resources, OPT_INVALID, false);
 
   // Collects the arguments for -fvk-{b|s|t|u}-shift.
@@ -519,6 +520,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
   }
 #else
   if (Args.hasFlag(OPT_spirv, OPT_INVALID, false) ||
+      Args.hasFlag(OPT_fvk_invert_y, OPT_INVALID, false) ||
       Args.hasFlag(OPT_fvk_ignore_unused_resources, OPT_INVALID, false) ||
       !Args.getLastArgValue(OPT_fvk_stage_io_order_EQ).empty() ||
       !Args.getLastArgValue(OPT_fvk_b_shift).empty() ||

tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h

@@ -19,6 +19,7 @@ struct EmitSPIRVOptions {
   bool codeGenHighLevel;
   bool defaultRowMajor;
   bool disableValidation;
+  bool invertY;
   bool ignoreUnusedResources;
   bool enable16BitTypes;
   llvm::StringRef stageIoOrder;

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

@@ -111,6 +111,13 @@ public:
   uint32_t createCompositeExtract(uint32_t resultType, uint32_t composite,
                                   llvm::ArrayRef<uint32_t> indexes);
 
+  /// \brief Creates a composite insert instruction. The given object will
+  /// replace the component in the composite at the given indices. Returns the
+  /// <result-id> for the new composite.
+  uint32_t createCompositeInsert(uint32_t resultType, uint32_t composite,
+                                 llvm::ArrayRef<uint32_t> indices,
+                                 uint32_t object);
+
   /// \brief Creates a vector shuffle instruction of selecting from the two
   /// vectors using selectors and returns the <result-id> of the result vector.
   uint32_t createVectorShuffle(uint32_t resultType, uint32_t vector1,
@@ -211,9 +218,9 @@ public:
   /// If compareVal is given a non-zero value, OpImageDrefGather or
   /// OpImageSparseDrefGather will be generated; otherwise, OpImageGather or
   /// OpImageSparseGather will be generated.
-  /// If residencyCodeId is not zero, the sparse version of the instructions will
-  /// be used, and the SPIR-V instruction for storing the resulting residency
-  /// code will also be emitted.
+  /// If residencyCodeId is not zero, the sparse version of the instructions
+  /// will be used, and the SPIR-V instruction for storing the resulting
+  /// residency code will also be emitted.
   uint32_t createImageGather(uint32_t texelType, uint32_t imageType,
                              uint32_t image, uint32_t sampler,
                              uint32_t coordinate, uint32_t component,

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -1343,6 +1343,18 @@ bool DeclResultIdMapper::writeBackOutputStream(const ValueDecl *decl,
     // We should have recorded its stage output variable previously.
     assert(found != stageVarIds.end());
 
+    // Negate SV_Position.y if requested
+    if (spirvOptions.invertY &&
+        semanticInfo.semantic->GetKind() == hlsl::Semantic::Kind::Position) {
+
+      const auto f32Type = theBuilder.getFloat32Type();
+      const auto v4f32Type = theBuilder.getVecType(f32Type, 4);
+      const auto oldY = theBuilder.createCompositeExtract(f32Type, value, {1});
+      const auto newY =
+          theBuilder.createUnaryOp(spv::Op::OpFNegate, f32Type, oldY);
+      value = theBuilder.createCompositeInsert(v4f32Type, value, {1}, newY);
+    }
+
     theBuilder.createStore(found->second, value);
     return true;
   }

tools/clang/lib/SPIRV/DeclResultIdMapper.h

@@ -599,7 +599,7 @@ DeclResultIdMapper::DeclResultIdMapper(const hlsl::ShaderModel &model,
       astContext(context), diags(context.getDiagnostics()),
       typeTranslator(context, builder, diags, options), entryFunctionId(0),
       needsLegalization(false),
-      glPerVertex(model, context, builder, typeTranslator) {}
+      glPerVertex(model, context, builder, typeTranslator, options.invertY) {}
 
 bool DeclResultIdMapper::decorateStageIOLocations() {
   // Try both input and output even if input location assignment failed

tools/clang/lib/SPIRV/GlPerVertex.cpp

@@ -57,12 +57,14 @@ inline bool hasGSPrimitiveTypeQualifier(const DeclaratorDecl *decl) {
 } // anonymous namespace
 
 GlPerVertex::GlPerVertex(const hlsl::ShaderModel &sm, ASTContext &context,
-                         ModuleBuilder &builder, TypeTranslator &translator)
+                         ModuleBuilder &builder, TypeTranslator &translator,
+                         bool negateY)
     : shaderModel(sm), astContext(context), theBuilder(builder),
-      typeTranslator(translator), inIsGrouped(true), outIsGrouped(true),
-      inBlockVar(0), outBlockVar(0), inClipVar(0), inCullVar(0), outClipVar(0),
-      outCullVar(0), inArraySize(0), outArraySize(0), inClipArraySize(1),
-      outClipArraySize(1), inCullArraySize(1), outCullArraySize(1) {}
+      typeTranslator(translator), invertY(negateY), inIsGrouped(true),
+      outIsGrouped(true), inBlockVar(0), outBlockVar(0), inClipVar(0),
+      inCullVar(0), outClipVar(0), outCullVar(0), inArraySize(0),
+      outArraySize(0), inClipArraySize(1), outClipArraySize(1),
+      inCullArraySize(1), outCullArraySize(1) {}
 
 void GlPerVertex::generateVars(uint32_t inArrayLen, uint32_t outArrayLen) {
   // Calling this method twice is an internal error.
@@ -624,8 +626,7 @@ bool GlPerVertex::readField(hlsl::Semantic::Kind semanticKind,
 }
 
 void GlPerVertex::writePositionOrPointSize(
-    bool isPosition, llvm::Optional<uint32_t> invocationId,
-    uint32_t value) const {
+    bool isPosition, llvm::Optional<uint32_t> invocationId, uint32_t value) {
   // We do not handle stand-alone Position/PointSize builtin here.
   assert(outIsGrouped);
 
@@ -643,6 +644,17 @@ void GlPerVertex::writePositionOrPointSize(
     // locate the Position/PointSize builtin.
     const uint32_t ptr =
         theBuilder.createAccessChain(ptrType, outBlockVar, {fieldIndex});
+
+    if (isPosition && invertY) {
+      if (shaderModel.IsVS() || shaderModel.IsDS()) {
+        const auto oldY =
+            theBuilder.createCompositeExtract(f32Type, value, {1});
+        const auto newY =
+            theBuilder.createUnaryOp(spv::Op::OpFNegate, f32Type, oldY);
+        value = theBuilder.createCompositeInsert(fieldType, value, {1}, newY);
+      }
+    }
+
     theBuilder.createStore(ptr, value);
     return;
   }
@@ -661,6 +673,7 @@ void GlPerVertex::writePositionOrPointSize(
   // and the second one is the struct index.
   const uint32_t ptr = theBuilder.createAccessChain(ptrType, outBlockVar,
                                                     {arrayIndex, fieldIndex});
+
   theBuilder.createStore(ptr, value);
 }
 

tools/clang/lib/SPIRV/GlPerVertex.h

@@ -57,7 +57,7 @@ namespace spirv {
 class GlPerVertex {
 public:
   GlPerVertex(const hlsl::ShaderModel &sm, ASTContext &context,
-              ModuleBuilder &builder, TypeTranslator &translator);
+              ModuleBuilder &builder, TypeTranslator &translator, bool negateY);
 
   /// Records a declaration of SV_ClipDistance/SV_CullDistance so later
   /// we can caculate the ClipDistance/CullDistance array layout.
@@ -140,7 +140,7 @@ private:
   /// Emits SPIR-V instructions for writing the Position/PointSize builtin.
   void writePositionOrPointSize(bool isPosition,
                                 llvm::Optional<uint32_t> invocationId,
-                                uint32_t value) const;
+                                uint32_t value);
   /// Emits SPIR-V instructions for writing data into the ClipDistance/
   /// CullDistance builtin starting from offset. The value to be written is
   /// fromValue, whose type is fromType. Necessary transformations will be
@@ -166,6 +166,10 @@ private:
   ModuleBuilder &theBuilder;
   TypeTranslator &typeTranslator;
 
+  /// Indicates whether to invert SV_Position.y to accommodate Vulkan's
+  /// coordinate system
+  bool invertY;
+
   /// We can have Position, ClipDistance, and CullDistance either grouped (G)
   /// into the gl_PerVertex struct, or separated (S) as stand-alone variables.
   /// The following table shows for each shader stage, which one is used:

tools/clang/lib/SPIRV/ModuleBuilder.cpp

@@ -148,6 +148,19 @@ ModuleBuilder::createCompositeExtract(uint32_t resultType, uint32_t composite,
   return resultId;
 }
 
+uint32_t ModuleBuilder::createCompositeInsert(uint32_t resultType,
+                                              uint32_t composite,
+                                              llvm::ArrayRef<uint32_t> indices,
+                                              uint32_t object) {
+  assert(insertPoint && "null insert point");
+  const uint32_t resultId = theContext.takeNextId();
+  instBuilder
+      .opCompositeInsert(resultType, resultId, object, composite, indices)
+      .x();
+  insertPoint->appendInstruction(std::move(constructSite));
+  return resultId;
+}
+
 uint32_t
 ModuleBuilder::createVectorShuffle(uint32_t resultType, uint32_t vector1,
                                    uint32_t vector2,
@@ -773,14 +786,13 @@ void ModuleBuilder::decorate(uint32_t targetId, spv::Decoration decoration) {
 }
 
 #define IMPL_GET_PRIMITIVE_TYPE(ty)                                            \
-  \
-uint32_t ModuleBuilder::get##ty##Type() {                                      \
+                                                                               \
+  uint32_t ModuleBuilder::get##ty##Type() {                                    \
     const Type *type = Type::get##ty(theContext);                              \
     const uint32_t typeId = theContext.getResultIdForType(type);               \
     theModule.addType(type, typeId);                                           \
     return typeId;                                                             \
-  \
-}
+  }
 
 IMPL_GET_PRIMITIVE_TYPE(Void)
 IMPL_GET_PRIMITIVE_TYPE(Bool)
@@ -1049,16 +1061,15 @@ uint32_t ModuleBuilder::getConstantBool(bool value) {
 }
 
 #define IMPL_GET_PRIMITIVE_CONST(builderTy, cppTy)                             \
-  \
-uint32_t ModuleBuilder::getConstant##builderTy(cppTy value) {                  \
+                                                                               \
+  uint32_t ModuleBuilder::getConstant##builderTy(cppTy value) {                \
     const uint32_t typeId = get##builderTy##Type();                            \
     const Constant *constant =                                                 \
         Constant::get##builderTy(theContext, typeId, value);                   \
     const uint32_t constId = theContext.getResultIdForConstant(constant);      \
     theModule.addConstant(constant, constId);                                  \
     return constId;                                                            \
-  \
-}
+  }
 
 IMPL_GET_PRIMITIVE_CONST(Int16, int16_t)
 IMPL_GET_PRIMITIVE_CONST(Int32, int32_t)

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -395,6 +395,9 @@ SPIRVEmitter::SPIRVEmitter(CompilerInstance &ci,
       seenPushConstantAt(), needsLegalization(false) {
   if (shaderModel.GetKind() == hlsl::ShaderModel::Kind::Invalid)
     emitError("unknown shader module: %0", {}) << shaderModel.GetName();
+  if (options.invertY &&
+      !(shaderModel.IsVS() || shaderModel.IsDS() || shaderModel.IsGS()))
+    emitError("-fvk-invert-y can only be used in VS/DS/GS", {});
 }
 
 void SPIRVEmitter::HandleTranslationUnit(ASTContext &context) {

tools/clang/test/CodeGenSPIRV/vk.cloption.invert-y.ds.hlsl

@@ -0,0 +1,33 @@
+// Run: %dxc -T ds_6_0 -E main -fvk-invert-y
+
+// HS PCF output
+struct HsPcfOut {
+  float  outTessFactor[4]   : SV_TessFactor;
+  float  inTessFactor[2]    : SV_InsideTessFactor;
+};
+
+// Per-vertex input structs
+struct DsCpIn {
+    float4 pos : SV_Position;
+};
+
+// Per-vertex output structs
+struct DsCpOut {
+    float4 pos : SV_Position;
+};
+
+[domain("quad")]
+DsCpOut main(OutputPatch<DsCpIn, 3> patch,
+             HsPcfOut pcfData) {
+  DsCpOut dsOut;
+  dsOut = (DsCpOut)0;
+  return dsOut;
+}
+
+// CHECK:      [[call:%\d+]] = OpFunctionCall %DsCpIn %src_main %param_var_patch %param_var_pcfData
+// CHECK-NEXT:  [[val:%\d+]] = OpCompositeExtract %v4float [[call]] 0
+// CHECK-NEXT:  [[ptr:%\d+]] = OpAccessChain %_ptr_Output_v4float %gl_PerVertexOut %uint_0
+// CHECK-NEXT: [[oldY:%\d+]] = OpCompositeExtract %float [[val]] 1
+// CHECK-NEXT: [[newY:%\d+]] = OpFNegate %float [[oldY]]
+// CHECK-NEXT:  [[pos:%\d+]] = OpCompositeInsert %v4float [[newY]] [[val]] 1
+// CHECK-NEXT:                 OpStore [[ptr]] [[pos]]

tools/clang/test/CodeGenSPIRV/vk.cloption.invert-y.gs.hlsl

@@ -0,0 +1,29 @@
+// Run: %dxc -T gs_6_0 -E main -fvk-invert-y
+
+// GS per-vertex input
+struct GsVIn {
+    float4 pos : SV_Position;
+};
+
+// GS per-vertex output
+struct GsVOut {
+    float4 pos : SV_Position;
+};
+
+[maxvertexcount(2)]
+void main(in    line GsVIn              inData[2],
+          inout      LineStream<GsVOut> outData) {
+
+    GsVOut vertex;
+    vertex = (GsVOut)0;
+// CHECK:      [[vert:%\d+]] = OpLoad %GsVIn %vertex
+// CHECK-NEXT:  [[val:%\d+]] = OpCompositeExtract %v4float [[vert]] 0
+// CHECK-NEXT: [[oldY:%\d+]] = OpCompositeExtract %float [[val]] 1
+// CHECK-NEXT: [[newY:%\d+]] = OpFNegate %float [[oldY]]
+// CHECK-NEXT:  [[pos:%\d+]] = OpCompositeInsert %v4float [[newY]] [[val]] 1
+// CHECK-NEXT:                 OpStore %gl_Position [[pos]]
+    outData.Append(vertex);
+
+    outData.RestartStrip();
+}
+

tools/clang/test/CodeGenSPIRV/vk.cloption.invert-y.vs.hlsl

@@ -0,0 +1,12 @@
+// Run: %dxc -T vs_6_0 -E main -fvk-invert-y
+
+float4 main(float4 a : A) : SV_Position {
+    return a;
+}
+
+// CHECK:         [[a:%\d+]] = OpFunctionCall %v4float %src_main %param_var_a
+// CHECK-NEXT:  [[ptr:%\d+]] = OpAccessChain %_ptr_Output_v4float %gl_PerVertexOut %uint_0
+// CHECK-NEXT: [[oldY:%\d+]] = OpCompositeExtract %float [[a]] 1
+// CHECK-NEXT: [[newY:%\d+]] = OpFNegate %float [[oldY]]
+// CHECK-NEXT:  [[pos:%\d+]] = OpCompositeInsert %v4float [[newY]] [[a]] 1
+// CHECK-NEXT:                 OpStore [[ptr]] [[pos]]

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -467,6 +467,7 @@ public:
 
           spirvOpts.codeGenHighLevel = opts.CodeGenHighLevel;
           spirvOpts.disableValidation = opts.DisableValidation;
+          spirvOpts.invertY = opts.VkInvertY;
           spirvOpts.ignoreUnusedResources = opts.VkIgnoreUnusedResources;
           spirvOpts.defaultRowMajor = opts.DefaultRowMajor;
           spirvOpts.stageIoOrder = opts.VkStageIoOrder;

tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -1023,6 +1023,16 @@ TEST_F(FileTest, VulkanCLOptionIgnoreUnusedResources) {
   runFileTest("vk.cloption.ignore-unused-resources.hlsl");
 }
 
+TEST_F(FileTest, VulkanCLOptionInvertYVS) {
+  runFileTest("vk.cloption.invert-y.vs.hlsl");
+}
+TEST_F(FileTest, VulkanCLOptionInvertYDS) {
+  runFileTest("vk.cloption.invert-y.ds.hlsl");
+}
+TEST_F(FileTest, VulkanCLOptionInvertYGS) {
+  runFileTest("vk.cloption.invert-y.gs.hlsl");
+}
+
 // Vulkan specific
 TEST_F(FileTest, VulkanLocation) { runFileTest("vk.location.hlsl"); }
 TEST_F(FileTest, VulkanLocationInputExplicitOutputImplicit) {