Merge branch 'main' into cbieneman/rewrite-out-params-new

.github/workflows/clang-format-checker.yml

@@ -12,6 +12,16 @@ jobs:
         with:
           fetch-depth: 2
 
+      - name: Find merge base
+        run: |
+        while ! git merge-base origin/$GITHUB_BASE_REF origin/$GITHUB_HEAD_REF > /dev/null; do
+          depth=$((depth+100))
+          git fetch --depth=$depth origin $GITHUB_BASE_REF && git fetch --depth=$depth origin $GITHUB_HEAD_REF
+          if [ "$depth" -gt "10000" ]; then
+            exit 1
+          fi
+        done
+
       - name: Get changed files
         id: changed-files
         uses: tj-actions/changed-files@v39

CONTRIBUTING.md

@@ -44,6 +44,8 @@ Here's a few things you should always do when making changes to the code base:
 
 The coding, style, and general engineering guidelines follow those described in the docs/CodingStandards.rst. For additional guidelines in code specific to HLSL, see the docs/HLSLChanges.rst file.
 
+DXC has adopted a clang-format requirement for all incoming changes. PRs to DXC should have the _changed code_ clang formatted to the LLVM style, and leave the remaining portions of the file unchanged. This can be done using the `git-clang-format` tool or IDE driven workflows. A GitHub action will run on all PRs to validate that the change is properly formatted.
+
 **Commit/Pull Request Format**
 
 ```

azure-pipelines.yml

@@ -34,10 +34,6 @@ stages:
         call utils\hct\hctstart.cmd %HLSL_SRC_DIR% %HLSL_BLD_DIR%
         call utils\hct\hcttest.cmd -$(configuration) noexec
       displayName: 'DXIL Tests'
-    - script: |
-        call utils\hct\hctstart.cmd %HLSL_SRC_DIR% %HLSL_BLD_DIR%
-        call utils\hct\hcttest.cmd -$(configuration) spirv_only
-      displayName: 'SPIRV Tests'
 
   - job: Nix
     timeoutInMinutes: 90

cmake/modules/HandleLLVMOptions.cmake

@@ -533,7 +533,11 @@ if(LLVM_USE_SANITIZER)
       message(WARNING "Unsupported value of LLVM_USE_SANITIZER: ${LLVM_USE_SANITIZER}")
     endif()
   else()
-    message(WARNING "LLVM_USE_SANITIZER is not supported on this platform.")
+    if (LLVM_USE_SANITIZER STREQUAL "Address")
+      append("-fsanitize=address" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+    else()
+      message(WARNING "Unsupported value of LLVM_USE_SANITIZER: ${LLVM_USE_SANITIZER}")
+    endif()
   endif()
   if (LLVM_USE_SANITIZE_COVERAGE)
     append("-fsanitize-coverage=edge,indirect-calls,8bit-counters,trace-cmp" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)

docs/SPIR-V.rst

@@ -297,10 +297,10 @@ Supported extensions
 * SPV_EXT_mesh_shader
 * SPV_EXT_shader_stencil_support
 * SPV_AMD_shader_early_and_late_fragment_tests
-* SPV_AMD_shader_explicit_vertex_parameter
 * SPV_GOOGLE_hlsl_functionality1
 * SPV_GOOGLE_user_type
 * SPV_NV_mesh_shader
+* SPV_KHR_fragment_shading_barycentric
 
 Vulkan specific attributes
 --------------------------
@@ -1583,7 +1583,7 @@ some system-value (SV) semantic strings will be translated into SPIR-V
 +---------------------------+-------------+----------------------------------------+-----------------------+-----------------------------+
 | SV_StencilRef             | PSOut       | ``FragStencilRefEXT``                  | N/A                   | ``StencilExportEXT``        |
 +---------------------------+-------------+----------------------------------------+-----------------------+-----------------------------+
-| SV_Barycentrics           | PSIn        | ``BaryCoord*AMD``                      | N/A                   | ``Shader``                  |
+| SV_Barycentrics           | PSIn        | ``BaryCoord*KHR``                      | N/A                   | ``FragmentBarycentricKHR``  |
 +---------------------------+-------------+----------------------------------------+-----------------------+-----------------------------+
 |                           | GSOut       | ``Layer``                              | N/A                   | ``Geometry``                |
 |                           +-------------+----------------------------------------+-----------------------+-----------------------------+

include/dxc/Support/microcom.h

@@ -74,7 +74,9 @@ template <typename T> void DxcCallDestructor(T *obj) { obj->T::~T(); }
 #define DXC_MICROCOM_REF_FIELD(m_dwRef)                                        \
   volatile std::atomic<llvm::sys::cas_flag> m_dwRef = {0};
 #define DXC_MICROCOM_ADDREF_IMPL(m_dwRef)                                      \
-  ULONG STDMETHODCALLTYPE AddRef() override { return (ULONG)++m_dwRef; }
+  ULONG STDMETHODCALLTYPE AddRef() noexcept override {                         \
+    return (ULONG)++m_dwRef;                                                   \
+  }
 #define DXC_MICROCOM_ADDREF_RELEASE_IMPL(m_dwRef)                              \
   DXC_MICROCOM_ADDREF_IMPL(m_dwRef)                                            \
   ULONG STDMETHODCALLTYPE Release() override {                                 \
@@ -107,7 +109,7 @@ inline T *CreateOnMalloc(IMalloc *pMalloc, Args &&...args) {
 
 #define DXC_MICROCOM_TM_ADDREF_RELEASE_IMPL()                                  \
   DXC_MICROCOM_ADDREF_IMPL(m_dwRef)                                            \
-  ULONG STDMETHODCALLTYPE Release() override {                                 \
+  ULONG STDMETHODCALLTYPE Release() noexcept override {                        \
     ULONG result = (ULONG)--m_dwRef;                                           \
     if (result == 0) {                                                         \
       CComPtr<IMalloc> pTmp(m_pMalloc);                                        \

include/dxc/Test/HlslTestUtils.h

@@ -19,7 +19,18 @@
 #include <string>
 #include <vector>
 #ifdef _WIN32
+
+// Disable -Wignored-qualifiers for WexTestClass.h.
+// For const size_t GetSize() const; in TestData.h.
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wignored-qualifiers"
+#endif
 #include "WexTestClass.h"
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+
 #include <dxgiformat.h>
 #else
 #include "WEXAdapter.h"

include/dxc/dxcpix.h

@@ -13,7 +13,9 @@
 #define __DXC_PIX__
 
 #include "dxc/dxcapi.h"
+#ifdef _WIN32
 #include "objidl.h"
+#endif
 
 struct __declspec(uuid("199d8c13-d312-4197-a2c1-07a532999727")) IDxcPixType
     : public IUnknown {

include/llvm/ADT/Triple.h

@@ -509,6 +509,12 @@ class Triple {
            getOS() == Triple::PS4;
   }
 
+  // HLSL Change Begin - Add DXIL Triple.
+  bool isDXIL() const {
+    return getArch() == Triple::dxil || getArch() == Triple::dxil64;
+  }
+  // HLSL Change End - Add DXIL Triple.
+
   /// @}
   /// @name Mutators
   /// @{

include/llvm/IR/Operator.h

@@ -501,6 +501,26 @@ class BitCastOperator
   }
 };
 
+// HLSL CHANGE: Add this helper class from upstream.
+class AddrSpaceCastOperator
+    : public ConcreteOperator<Operator, Instruction::AddrSpaceCast> {
+  friend class AddrSpaceCastInst;
+  friend class ConstantExpr;
+
+public:
+  Value *getPointerOperand() { return getOperand(0); }
+
+  const Value *getPointerOperand() const { return getOperand(0); }
+
+  unsigned getSrcAddressSpace() const {
+    return getPointerOperand()->getType()->getPointerAddressSpace();
+  }
+
+  unsigned getDestAddressSpace() const {
+    return getType()->getPointerAddressSpace();
+  }
+};
+
 } // End llvm namespace
 
 #endif

lib/CMakeLists.txt

@@ -27,9 +27,7 @@ add_subdirectory(DXIL) # HLSL Change
 add_subdirectory(DxilContainer) # HLSL Change
 add_subdirectory(DxilPdbInfo) # HLSL Change
 add_subdirectory(DxilPIXPasses) # HLSL Change
-if(WIN32) # HLSL Change
-  add_subdirectory(DxilDia) # HLSL Change
-endif(WIN32) # HLSL Change
+add_subdirectory(DxilDia) # HLSL Change
 add_subdirectory(DxilRootSignature) # HLSL Change
 add_subdirectory(DxcBindingTable) # HLSL Change
 add_subdirectory(DxrFallback) # HLSL Change

lib/DXIL/DxilOperations.cpp

@@ -685,7 +685,7 @@ const OP::OpCodeProperty OP::m_OpCodeProps[(unsigned)OP::OpCode::NumOpCodes] = {
         "SampleCmp",
         OCC::SampleCmp,
         "sampleCmp",
-        {false, true, true, false, false, false, true, true, false, false,
+        {false, true, true, false, false, false, false, false, false, false,
          false},
         Attribute::ReadOnly,
     },
@@ -694,7 +694,7 @@ const OP::OpCodeProperty OP::m_OpCodeProps[(unsigned)OP::OpCode::NumOpCodes] = {
         "SampleCmpLevelZero",
         OCC::SampleCmpLevelZero,
         "sampleCmpLevelZero",
-        {false, true, true, false, false, false, true, true, false, false,
+        {false, true, true, false, false, false, false, false, false, false,
          false},
         Attribute::ReadOnly,
     },

lib/DXIL/DxilSignatureElement.cpp

@@ -51,6 +51,9 @@ void DxilSignatureElement::Initialize(llvm::StringRef Name,
     m_SemanticStartIndex = IndexVector[0];
   // Find semantic in the table.
   m_pSemantic = Semantic::GetByName(m_SemanticName, m_sigPointKind);
+  // Replace semantic name with canonical name if it's a system value.
+  if (!m_pSemantic->IsInvalid() && !m_pSemantic->IsArbitrary())
+    m_SemanticName = m_pSemantic->GetName();
   SetCompType(ElementType);
   m_InterpMode = InterpMode;
   m_SemanticIndex = IndexVector;

lib/DXIL/DxilUtilDbgInfoAndMisc.cpp

@@ -37,7 +37,18 @@ using namespace hlsl;
 
 namespace {
 
-Value *MergeGEP(GEPOperator *SrcGEP, GEPOperator *GEP) {
+// Attempt to merge the two GEPs into a single GEP.
+//
+// If `AsCast` is non-null the merged GEP will be wrapped
+// in an addrspacecast before replacing users. This allows
+// merging GEPs of the form
+//
+//      gep(addrspacecast(gep(p0, gep_args0) to p1*), gep_args1)
+// into
+//      addrspacecast(gep(p0, gep_args0+gep_args1) to p1*)
+//
+Value *MergeGEP(GEPOperator *SrcGEP, GEPOperator *GEP,
+                AddrSpaceCastOperator *AsCast) {
   IRBuilder<> Builder(GEP->getContext());
   StringRef Name = "";
   if (Instruction *I = dyn_cast<Instruction>(GEP)) {
@@ -75,7 +86,7 @@ Value *MergeGEP(GEPOperator *SrcGEP, GEPOperator *GEP) {
     }
 
     // Update the GEP in place if possible.
-    if (SrcGEP->getNumOperands() == 2) {
+    if (SrcGEP->getNumOperands() == 2 && !AsCast) {
       GEP->setOperand(0, SrcGEP->getOperand(0));
       GEP->setOperand(1, Sum);
       return GEP;
@@ -94,12 +105,64 @@ Value *MergeGEP(GEPOperator *SrcGEP, GEPOperator *GEP) {
   DXASSERT(!Indices.empty(), "must merge");
   Value *newGEP =
       Builder.CreateInBoundsGEP(nullptr, SrcGEP->getOperand(0), Indices, Name);
+
+  // Wrap the new gep in an addrspacecast if needed.
+  if (AsCast)
+    newGEP = Builder.CreateAddrSpaceCast(
+        newGEP, PointerType::get(GEP->getType()->getPointerElementType(),
+                                 AsCast->getDestAddressSpace()));
   GEP->replaceAllUsesWith(newGEP);
   if (Instruction *I = dyn_cast<Instruction>(GEP))
     I->eraseFromParent();
   return newGEP;
 }
 
+// Examine the gep and try to merge it when the input pointer is
+// itself a gep. We handle two forms here:
+//
+//      gep(gep(p))
+//      gep(addrspacecast(gep(p)))
+//
+// If the gep was merged successfully then return the updated value, otherwise
+// return nullptr.
+//
+// When the gep is sucessfully merged we will delete the gep and also try to
+// delete the nested gep and addrspacecast.
+static Value *TryMegeWithNestedGEP(GEPOperator *GEP) {
+  // Sentinal value to return when we fail to merge.
+  Value *FailedToMerge = nullptr;
+
+  Value *Ptr = GEP->getPointerOperand();
+  GEPOperator *prevGEP = dyn_cast<GEPOperator>(Ptr);
+  AddrSpaceCastOperator *AsCast = nullptr;
+
+  // If there is no directly nested gep try looking through an addrspacecast to
+  // find one.
+  if (!prevGEP) {
+    AsCast = dyn_cast<AddrSpaceCastOperator>(Ptr);
+    if (AsCast)
+      prevGEP = dyn_cast<GEPOperator>(AsCast->getPointerOperand());
+  }
+
+  // Not a nested gep expression.
+  if (!prevGEP)
+    return FailedToMerge;
+
+  // Try merging the two geps.
+  Value *newGEP = MergeGEP(prevGEP, GEP, AsCast);
+  if (!newGEP)
+    return FailedToMerge;
+
+  // Delete the nested gep and addrspacecast if no more users.
+  if (AsCast && AsCast->user_empty() && isa<AddrSpaceCastInst>(AsCast))
+    cast<AddrSpaceCastInst>(AsCast)->eraseFromParent();
+
+  if (prevGEP->user_empty() && isa<GetElementPtrInst>(prevGEP))
+    cast<GetElementPtrInst>(prevGEP)->eraseFromParent();
+
+  return newGEP;
+}
+
 } // namespace
 
 namespace hlsl {
@@ -130,23 +193,14 @@ bool MergeGepUse(Value *V) {
         // merge any GEP users of the untranslated bitcast
         addUsersToWorklist(V);
       }
+    } else if (isa<AddrSpaceCastOperator>(V)) {
+      addUsersToWorklist(V);
     } else if (GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
-      if (GEPOperator *prevGEP =
-              dyn_cast<GEPOperator>(GEP->getPointerOperand())) {
-        // merge the 2 GEPs, returns nullptr if couldn't merge
-        if (Value *newGEP = MergeGEP(prevGEP, GEP)) {
-          changed = true;
-          worklist.push_back(newGEP);
-          // delete prevGEP if no more users
-          if (prevGEP->user_empty() && isa<GetElementPtrInst>(prevGEP)) {
-            cast<GetElementPtrInst>(prevGEP)->eraseFromParent();
-          }
-        } else {
-          addUsersToWorklist(GEP);
-        }
+      if (Value *newGEP = TryMegeWithNestedGEP(GEP)) {
+        changed = true;
+        worklist.push_back(newGEP);
       } else {
-        // nothing to merge yet, add GEP users
-        addUsersToWorklist(V);
+        addUsersToWorklist(GEP);
       }
     }
   }