Disallow illegal Atomic targets + propagate consts (#4453)

This change does a few things.

First and foremost, it disallows a number of illegal destination
parameters to atomic operations. This includes SRVs and other const
 values, non-groupshared and non-resource variables, members of
typed resources, and bitfield members of any resource.

In addition, this correctly propagates const information so that they
can be properly rejected and also the information is reflected.
This involves the consolidation and earlier detection of a number of
these failures as well as an expansion of that detection.
Also adds validation checks that the targets are not const and either
UAVs or groupshared address space.

The const errors are produced in clang and manifest as overload
mismatches. The errors for member access of typed buffers, bitfields,
or non-groupshared and non-UAV destinations are generated
as part of the lowering operation. This is where they were before
and easier to replicate though they would be better in codegen.
In some cases, existing errors were moved earlier in the process
and the logic to generate them simplified.

Add compilation and verifier tests for all.

Also adds validation errors for const destination targets and other
invalid targets and tests for the same.

In order to make those tests, I had to give locations to the function
candidate notes for function builtins. This changed a single verifier
test that depended on the locationless notes.

These tests make use of const groupshared variables that are of
questionable utility, but are valid for now.

Incidentally enhances some existing atomics tests to use a more
complex sequence of references and subscripts.

Finally incidentally added some foolproofing for mistakenly comparing
opcodes without taking into account the "table" or namespace that
they belonged to by changing some utility functions in Sema to require
the table as well as the opcode to produce results.

Fixes #4319
Fixes #4377
Fixes #4437

docs/DXIL.rst

@@ -2997,6 +2997,9 @@ FLOW.FUNCTIONCALL                         Function with parameter is not permitt
 FLOW.NORECUSION                           Recursion is not permitted.
 FLOW.REDUCIBLE                            Execution flow must be reducible.
 INSTR.ALLOWED                             Instructions must be of an allowed type.
+INSTR.ATOMICCONST                         Constant destination to atomic.
+INSTR.ATOMICINTRINNONUAV                  Non-UAV destination to atomic intrinsic.
+INSTR.ATOMICOPNONGROUPSHARED              Non-groupshared destination to atomic operation.
 INSTR.ATTRIBUTEATVERTEXNOINTERPOLATION    Attribute %0 must have nointerpolation mode in order to use GetAttributeAtVertex function.
 INSTR.BARRIERMODEFORNONCS                 sync in a non-Compute/Amplification/Mesh Shader must only sync UAV (sync_uglobal).
 INSTR.BARRIERMODENOMEMORY                 sync must include some form of memory barrier - _u (UAV) and/or _g (Thread Group Shared Memory).  Only _t (thread group sync) is optional.

include/dxc/HLSL/HLOperations.h

@@ -132,7 +132,6 @@ void SetHLWaveSensitive(llvm::Function *F);
 bool IsHLWaveSensitive(llvm::Function *F);
 
 // For intrinsic opcode.
-bool HasUnsignedOpcode(unsigned opcode);
 unsigned GetUnsignedOpcode(unsigned opcode);
 // For HLBinaryOpcode.
 bool HasUnsignedOpcode(HLBinaryOpcode opcode);

lib/HLSL/DxilValidation.cpp

@@ -2192,6 +2192,10 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
     if ((pOverloadType->isIntegerTy(64)) && !pSM->IsSM66Plus())
       ValCtx.EmitInstrFormatError(CI, ValidationRule::SmOpcodeInInvalidFunction,
                                   {"64-bit atomic operations", "Shader Model 6.6+"});
+    Value *Handle = CI->getOperand(DXIL::OperandIndex::kAtomicBinOpHandleOpIdx);
+    if (!isa<CallInst>(Handle) ||
+        ValCtx.GetResourceFromVal(Handle).getResourceClass() != DXIL::ResourceClass::UAV)
+      ValCtx.EmitInstrError(CI, ValidationRule::InstrAtomicIntrinNonUAV);
   } break;
   case DXIL::OpCode::CreateHandle:
     if (ValCtx.isLibProfile) {
@@ -3180,11 +3184,34 @@ static void ValidateFunctionBody(Function *F, ValidationContext &ValCtx) {
       } break;
       case Instruction::AtomicCmpXchg:
       case Instruction::AtomicRMW: {
-        Type *T = cast<PointerType>(I.getOperand(AtomicRMWInst::getPointerOperandIndex())->getType())->getElementType();
+        Value *Ptr = I.getOperand(AtomicRMWInst::getPointerOperandIndex());
+        PointerType *ptrType = cast<PointerType>(Ptr->getType());
+        Type *elType = ptrType->getElementType();
         const ShaderModel *pSM = ValCtx.DxilMod.GetShaderModel();
-        if ((T->isIntegerTy(64)) && !pSM->IsSM66Plus())
+        if ((elType->isIntegerTy(64)) && !pSM->IsSM66Plus())
           ValCtx.EmitInstrFormatError(&I, ValidationRule::SmOpcodeInInvalidFunction,
                                       {"64-bit atomic operations", "Shader Model 6.6+"});
+
+        if (ptrType->getAddressSpace() != DXIL::kTGSMAddrSpace)
+          ValCtx.EmitInstrError(&I, ValidationRule::InstrAtomicOpNonGroupshared);
+
+        // Drill through GEP and bitcasts
+        while (true) {
+          if (GEPOperator *GEP = dyn_cast<GEPOperator>(Ptr)) {
+            Ptr = GEP->getPointerOperand();
+            continue;
+          }
+          if (BitCastInst *BC = dyn_cast<BitCastInst>(Ptr)) {
+            Ptr = BC->getOperand(0);
+            continue;
+          }
+          break;
+        }
+
+        if (GlobalVariable *GV = dyn_cast<GlobalVariable>(Ptr)) {
+          if(GV->isConstant())
+            ValCtx.EmitInstrError(&I, ValidationRule::InstrAtomicConst);
+        }
       } break;
 
       }

lib/HLSL/HLOperationLower.cpp

@@ -4421,6 +4421,44 @@ static Value* SkipAddrSpaceCast(Value* Ptr) {
   return Ptr;
 }
 
+// For known non-groupshared, verify that the destination param is valid
+void ValidateAtomicDestination(CallInst *CI, HLObjectOperationLowerHelper *pObjHelper) {
+  Value *dest = CI->getArgOperand(HLOperandIndex::kInterlockedDestOpIndex);
+  // If we encounter a gep, we may provide a more specific error message
+  bool hasGep = isa<GetElementPtrInst>(dest);
+
+  // Confirm that dest is a properly-used UAV
+
+  // Drill through subscripts and geps, anything else indicates a misuse
+  while(true) {
+    if (GetElementPtrInst *gep = dyn_cast<GetElementPtrInst>(dest)) {
+      dest = gep->getPointerOperand();
+      continue;
+    }
+    if (CallInst *handle = dyn_cast<CallInst>(dest)) {
+      hlsl::HLOpcodeGroup group = hlsl::GetHLOpcodeGroup(handle->getCalledFunction());
+      if (group != HLOpcodeGroup::HLSubscript)
+        break;
+      dest = handle->getArgOperand(HLOperandIndex::kSubscriptObjectOpIdx);
+      continue;
+    }
+    break;
+  }
+
+  if(pObjHelper->GetRC(dest) == DXIL::ResourceClass::UAV) {
+    DXIL::ResourceKind RK = pObjHelper->GetRK(dest);
+    if (DXIL::IsStructuredBuffer(RK))
+      return; // no errors
+    if (DXIL::IsTyped(RK)) {
+      if (hasGep)
+        dxilutil::EmitErrorOnInstruction(CI, "Typed resources used in atomic operations must have a scalar element type.");
+      return; // error emitted or else no errors
+    }
+  }
+
+  dxilutil::EmitErrorOnInstruction(CI, "Atomic operation targets must be groupshared or UAV.");
+}
+
 Value *TranslateIopAtomicBinaryOperation(CallInst *CI, IntrinsicOp IOP,
                                          DXIL::OpCode opcode,
                                          HLOperationLowerHelper &helper,  HLObjectOperationLowerHelper *pObjHelper, bool &Translated) {
@@ -4431,11 +4469,13 @@ Value *TranslateIopAtomicBinaryOperation(CallInst *CI, IntrinsicOp IOP,
   if (addressSpace == DXIL::kTGSMAddrSpace)
     TranslateSharedMemAtomicBinOp(CI, IOP, addr);
   else {
-    // buffer atomic translated in TranslateSubscript.
-    // Do nothing here.
-    // Mark not translated.
+    // If not groupshared, we either have an error case or will translate
+    // the atomic op in the process of translating users of the subscript operator
+    // Mark not translated and validate dest param
     Translated = false;
+    ValidateAtomicDestination(CI, pObjHelper);
   }
+
   return nullptr;
 }
 
@@ -4480,10 +4520,11 @@ Value *TranslateIopAtomicCmpXChg(CallInst *CI, IntrinsicOp IOP,
   if (addressSpace == DXIL::kTGSMAddrSpace)
     TranslateSharedMemAtomicCmpXChg(CI, addr);
   else {
-    // buffer atomic translated in TranslateSubscript.
-    // Do nothing here.
-    // Mark not translated.
+    // If not groupshared, we either have an error case or will translate
+    // the atomic op in the process of translating users of the subscript operator
+    // Mark not translated and validate dest param
     Translated = false;
+    ValidateAtomicDestination(CI, pObjHelper);
   }
 
   return nullptr;
@@ -7698,50 +7739,10 @@ void TranslateDefaultSubscript(CallInst *CI, HLOperationLowerHelper &helper,  HL
           LI->eraseFromParent();
           continue;
         }
-        if (!isa<CallInst>(GEPUser)) {
-          // Invalid operations.
-          Translated = false;
-          dxilutil::EmitErrorOnInstruction(GEP, "Invalid operation on typed buffer.");
-          return;
-        }
-        CallInst *userCall = cast<CallInst>(GEPUser);
-        HLOpcodeGroup group =
-            hlsl::GetHLOpcodeGroupByName(userCall->getCalledFunction());
-        if (group != HLOpcodeGroup::HLIntrinsic) {
-          // Invalid operations.
-          Translated = false;
-          dxilutil::EmitErrorOnInstruction(userCall, "Invalid operation on typed buffer.");
-          return;
-        }
-        unsigned opcode = hlsl::GetHLOpcode(userCall);
-        IntrinsicOp IOP = static_cast<IntrinsicOp>(opcode);
-        switch (IOP) {
-        case IntrinsicOp::IOP_InterlockedAdd:
-        case IntrinsicOp::IOP_InterlockedAnd:
-        case IntrinsicOp::IOP_InterlockedExchange:
-        case IntrinsicOp::IOP_InterlockedMax:
-        case IntrinsicOp::IOP_InterlockedMin:
-        case IntrinsicOp::IOP_InterlockedUMax:
-        case IntrinsicOp::IOP_InterlockedUMin:
-        case IntrinsicOp::IOP_InterlockedOr:
-        case IntrinsicOp::IOP_InterlockedXor:
-        case IntrinsicOp::IOP_InterlockedCompareStore:
-        case IntrinsicOp::IOP_InterlockedCompareExchange:
-        case IntrinsicOp::IOP_InterlockedCompareStoreFloatBitwise:
-        case IntrinsicOp::IOP_InterlockedCompareExchangeFloatBitwise: {
-          // Invalid operations.
-          Translated = false;
-          dxilutil::EmitErrorOnInstruction(
-              userCall, "Typed resources used in atomic operations must have a scalar element type.");
-          return;
-        } break;
-        default:
-          // Invalid operations.
-          Translated = false;
-          dxilutil::EmitErrorOnInstruction(userCall, "Invalid operation on typed buffer.");
-          return;
-          break;
-        }
+        // Invalid operations.
+        Translated = false;
+        dxilutil::EmitErrorOnInstruction(GEP, "Invalid operation on typed buffer.");
+        return;
       }
       GEP->eraseFromParent();
     } else {
@@ -7754,29 +7755,8 @@ void TranslateDefaultSubscript(CallInst *CI, HLOperationLowerHelper &helper,  HL
         if (RC == DXIL::ResourceClass::SRV) {
           // Invalid operations.
           Translated = false;
-          switch (IOP) {
-          case IntrinsicOp::IOP_InterlockedAdd:
-          case IntrinsicOp::IOP_InterlockedAnd:
-          case IntrinsicOp::IOP_InterlockedExchange:
-          case IntrinsicOp::IOP_InterlockedMax:
-          case IntrinsicOp::IOP_InterlockedMin:
-          case IntrinsicOp::IOP_InterlockedUMax:
-          case IntrinsicOp::IOP_InterlockedUMin:
-          case IntrinsicOp::IOP_InterlockedOr:
-          case IntrinsicOp::IOP_InterlockedXor:
-          case IntrinsicOp::IOP_InterlockedCompareStore:
-          case IntrinsicOp::IOP_InterlockedCompareExchange:
-          case IntrinsicOp::IOP_InterlockedCompareStoreFloatBitwise:
-          case IntrinsicOp::IOP_InterlockedCompareExchangeFloatBitwise: {
-            dxilutil::EmitErrorOnInstruction(
-                userCall, "Atomic operation targets must be groupshared or UAV.");
-            return;
-          } break;
-          default:
-            dxilutil::EmitErrorOnInstruction(userCall, "Invalid operation on typed buffer.");
-            return;
-            break;
-          }
+          dxilutil::EmitErrorOnInstruction(userCall, "Invalid operation on SRV.");
+          return;
         }
         switch (IOP) {
         case IntrinsicOp::IOP_InterlockedAdd: {

lib/HLSL/HLOperations.cpp

@@ -374,10 +374,6 @@ unsigned  GetRowMajorOpcode(HLOpcodeGroup group, unsigned opcode) {
   }
 }
 
-bool HasUnsignedOpcode(unsigned opcode) {
-  return HasUnsignedIntrinsicOpcode(static_cast<IntrinsicOp>(opcode));
-}
-
 unsigned GetUnsignedOpcode(unsigned opcode) {
   return GetUnsignedIntrinsicOpcode(static_cast<IntrinsicOp>(opcode));
 }

tools/clang/lib/Sema/SemaExpr.cpp

@@ -4285,7 +4285,7 @@ Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, SourceLocation LLoc,
       // We need to make sure to preserve qualifiers on array types, since these
       // are in effect references.
       if (LHSTy.hasQualifiers())
-        ResultType.setLocalFastQualifiers(LHSTy.getLocalFastQualifiers());
+        ResultType.setLocalFastQualifiers(LHSTy.getQualifiers().getFastQualifiers());
     } else {
     // HLSL Change Ends
       Diag(LHSExp->getLocStart(), diag::ext_subscript_non_lvalue) <<

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -1858,7 +1858,13 @@ static void InitParamMods(const HLSL_INTRINSIC *pIntrinsic,
   }
 }
 
-static bool IsAtomicOperation(IntrinsicOp op) {
+static bool IsBuiltinTable(LPCSTR tableName) {
+  return tableName == kBuiltinIntrinsicTableName;
+}
+
+static bool IsAtomicOperation(LPCSTR tableName, IntrinsicOp op) {
+  if (!IsBuiltinTable(tableName))
+    return false;
   switch (op) {
   case IntrinsicOp::IOP_InterlockedAdd:
   case IntrinsicOp::IOP_InterlockedAnd:
@@ -1898,15 +1904,15 @@ static bool IsAtomicOperation(IntrinsicOp op) {
   }
 }
 
-static bool IsBuiltinTable(LPCSTR tableName) {
-  return tableName == kBuiltinIntrinsicTableName;
+static bool HasUnsignedOpcode(LPCSTR tableName, IntrinsicOp opcode) {
+  return IsBuiltinTable(tableName) && HasUnsignedIntrinsicOpcode(opcode);
 }
 
 static void AddHLSLIntrinsicAttr(FunctionDecl *FD, ASTContext &context,
                               LPCSTR tableName, LPCSTR lowering,
                               const HLSL_INTRINSIC *pIntrinsic) {
-  unsigned opcode = (unsigned)pIntrinsic->Op;
-  if (HasUnsignedOpcode(opcode) && IsBuiltinTable(tableName)) {
+  unsigned opcode = pIntrinsic->Op;
+  if (HasUnsignedOpcode(tableName, static_cast<IntrinsicOp>(opcode))) {
     QualType Ty = FD->getReturnType();
     if (pIntrinsic->iOverloadParamIndex != -1) {
       const FunctionProtoType *FT =
@@ -1965,13 +1971,11 @@ FunctionDecl *AddHLSLIntrinsicFunction(
   InitParamMods(pIntrinsic, paramMods);
 
   // Change dest address into reference type for atomic.
-  if (IsBuiltinTable(tableName)) {
-    if (IsAtomicOperation(static_cast<IntrinsicOp>(pIntrinsic->Op))) {
-      DXASSERT(functionArgTypeCount > kAtomicDstOperandIdx,
-               "else operation was misrecognized");
-      functionArgQualTypes[kAtomicDstOperandIdx] =
-          context.getLValueReferenceType(functionArgQualTypes[kAtomicDstOperandIdx]);
-    }
+  if (IsAtomicOperation(tableName, static_cast<IntrinsicOp>(pIntrinsic->Op))) {
+    DXASSERT(functionArgTypeCount > kAtomicDstOperandIdx,
+             "else operation was misrecognized");
+    functionArgQualTypes[kAtomicDstOperandIdx] =
+      context.getLValueReferenceType(functionArgQualTypes[kAtomicDstOperandIdx]);
   }
 
   for (size_t i = 1; i < functionArgTypeCount; i++) {
@@ -2583,13 +2587,13 @@ public:
     return _tableIndex != other._tableIndex; // More things could be compared but we only match end.
   }
 
-  const HLSL_INTRINSIC* operator*()
+  const HLSL_INTRINSIC* operator*() const
   {
     DXASSERT(_firstChecked, "otherwise deref without comparing to end");
     return _tableIntrinsic;
   }
 
-  LPCSTR GetTableName()
+  LPCSTR GetTableName() const
   {
     LPCSTR tableName = nullptr;
     if (FAILED(_tables[_tableIndex]->GetTableName(&tableName))) {
@@ -2598,7 +2602,7 @@ public:
     return tableName;
   }
 
-  LPCSTR GetLoweringStrategy()
+  LPCSTR GetLoweringStrategy() const
   {
     LPCSTR lowering = nullptr;
     if (FAILED(_tables[_tableIndex]->GetLoweringStrategy(_tableIntrinsic->Op, &lowering))) {
@@ -2643,17 +2647,17 @@ public:
     return _current != other._current || _tableIter.operator!=(other._tableIter);
   }
 
-  const HLSL_INTRINSIC* operator*()
+  const HLSL_INTRINSIC* operator*() const
   {
     return (_current != _end) ? _current : *_tableIter;
   }
 
-  LPCSTR GetTableName()
+  LPCSTR GetTableName() const
   {
     return (_current != _end) ? kBuiltinIntrinsicTableName : _tableIter.GetTableName();
   }
 
-  LPCSTR GetLoweringStrategy()
+  LPCSTR GetLoweringStrategy() const
   {
     return (_current != _end) ? "" : _tableIter.GetLoweringStrategy();
   }
@@ -4638,14 +4642,14 @@ public:
   }
 
   /// <summary>Attempts to match Args to the signature specification in pIntrinsic.</summary>
-  /// <param name="pIntrinsic">Intrinsic function to match.</param>
+  /// <param name="cursor">Intrinsic function iterator.</param>
   /// <param name="objectElement">Type element on the class intrinsic belongs to; possibly null (eg, 'float' in 'Texture2D<float>').</param>
   /// <param name="Args">Invocation arguments to match.</param>
   /// <param name="argTypes">After exectuion, type of arguments.</param>
   /// <param name="badArgIdx">The first argument to mismatch if any</param>
   /// <remarks>On success, argTypes includes the clang Types to use for the signature, with the first being the return type.</remarks>
   bool MatchArguments(
-    const _In_ HLSL_INTRINSIC *pIntrinsic,
+    const IntrinsicDefIter &cursor,
     _In_ QualType objectElement,
     _In_ QualType functionTemplateTypeArg,
     _In_ ArrayRef<Expr *> Args, 
@@ -4653,10 +4657,12 @@ public:
     _Out_ size_t &badArgIdx);
 
   /// <summary>Validate object element on intrinsic to catch case like integer on Sample.</summary>
-  /// <param name="pIntrinsic">Intrinsic function to validate.</param>
+  /// <param name="tableName">Intrinsic function to validate.</param>
+  /// <param name="op">Intrinsic opcode to validate.</param>
   /// <param name="objectElement">Type element on the class intrinsic belongs to; possibly null (eg, 'float' in 'Texture2D<float>').</param>
   bool IsValidObjectElement(
-    _In_ const HLSL_INTRINSIC *pIntrinsic,
+    LPCSTR tableName,
+    _In_ IntrinsicOp op,
     _In_ QualType objectElement);
 
   // Returns the iterator with the first entry that matches the requirement
@@ -4768,7 +4774,7 @@ public:
 
       std::vector<QualType> functionArgTypes;
       size_t badArgIdx;
-      bool argsMatch = MatchArguments(pIntrinsic, QualType(), QualType(), Args, &functionArgTypes, badArgIdx);
+      bool argsMatch = MatchArguments(cursor, QualType(), QualType(), Args, &functionArgTypes, badArgIdx);
       if (!functionArgTypes.size())
         return false;
 
@@ -5900,9 +5906,11 @@ ConcreteLiteralType(Expr *litExpr, ArBasicKind kind,
 }
 
 _Use_decl_annotations_ bool
-HLSLExternalSource::IsValidObjectElement(const HLSL_INTRINSIC *pIntrinsic,
+HLSLExternalSource::IsValidObjectElement(LPCSTR tableName, const IntrinsicOp op,
                                          QualType objectElement) {
-  IntrinsicOp op = static_cast<IntrinsicOp>(pIntrinsic->Op);
+  // Only meant to exclude builtins, assume others are fine
+  if (!IsBuiltinTable(tableName))
+    return true;
   switch (op) {
   case IntrinsicOp::MOP_Sample:
   case IntrinsicOp::MOP_SampleBias:
@@ -5936,13 +5944,19 @@ HLSLExternalSource::IsValidObjectElement(const HLSL_INTRINSIC *pIntrinsic,
 
 _Use_decl_annotations_
 bool HLSLExternalSource::MatchArguments(
-  const HLSL_INTRINSIC* pIntrinsic,
+  const IntrinsicDefIter &cursor,
   QualType objectElement,
   QualType functionTemplateTypeArg,
   ArrayRef<Expr *> Args,
   std::vector<QualType> *argTypesVector,
   size_t &badArgIdx)
 {
+  const HLSL_INTRINSIC* pIntrinsic = *cursor;
+  LPCSTR tableName = cursor.GetTableName();
+  IntrinsicOp builtinOp = IntrinsicOp::Num_Intrinsics;
+  if (IsBuiltinTable(tableName))
+    builtinOp = static_cast<IntrinsicOp>(pIntrinsic->Op);
+
   DXASSERT_NOMSG(pIntrinsic != nullptr);
   DXASSERT_NOMSG(argTypesVector != nullptr);
   std::vector<QualType> &argTypes = *argTypesVector;
@@ -6155,14 +6169,23 @@ bool HLSLExternalSource::MatchArguments(
       }
     }
 
+    ASTContext &actx = m_sema->getASTContext();
     // Usage
     if (pIntrinsicArg->qwUsage & AR_QUAL_OUT) {
-      if (pCallArg->getType().isConstQualified()) {
+      if (pType.isConstant(actx)) {
         // Can't use a const type in an out or inout parameter.
         badArgIdx = std::min(badArgIdx, iArg);
       }
     }
 
+    // Catch invalid atomic dest parameters
+    if (iArg == kAtomicDstOperandIdx &&
+        IsAtomicOperation(tableName, builtinOp)) {
+      // This produces an error for bitfields that is a bit confusing because it says uint can't cast to uint
+      if (pType.isConstant(actx) || pCallArg->getObjectKind() == OK_BitField) {
+        badArgIdx = std::min(badArgIdx, iArg);
+      }
+    }
     iArg++;
   }
 
@@ -6359,7 +6382,7 @@ bool HLSLExternalSource::MatchArguments(
         if (i == 0) {
           // [RW]ByteAddressBuffer.Load, default to uint
           pNewType = m_context->UnsignedIntTy;
-          if (pIntrinsic->Op != (UINT)hlsl::IntrinsicOp::MOP_Load)
+          if (builtinOp != hlsl::IntrinsicOp::MOP_Load)
             badArgIdx = std::min(badArgIdx, i);
         }
         else {
@@ -9985,7 +10008,7 @@ Sema::TemplateDeductionResult HLSLExternalSource::DeduceTemplateArgumentsForHLSL
   while (cursor != end)
   {
     size_t badArgIdx;
-    if (!MatchArguments(*cursor, objectElement, functionTemplateTypeArg, Args, &argTypes, badArgIdx))
+    if (!MatchArguments(cursor, objectElement, functionTemplateTypeArg, Args, &argTypes, badArgIdx))
     {
       ++cursor;
       continue;
@@ -10046,7 +10069,8 @@ Sema::TemplateDeductionResult HLSLExternalSource::DeduceTemplateArgumentsForHLSL
     DXASSERT_NOMSG(Specialization->getPrimaryTemplate()->getCanonicalDecl() ==
       FunctionTemplate->getCanonicalDecl());
 
-    if (IsBuiltinTable(tableName) && !IsValidObjectElement(*cursor, objectElement)) {
+    const HLSL_INTRINSIC* pIntrinsic = *cursor;
+    if (!IsValidObjectElement(tableName, static_cast<IntrinsicOp>(pIntrinsic->Op), objectElement)) {
       UINT numEles = GetNumElements(objectElement);
       std::string typeName(g_ArBasicTypeNames[GetTypeElementKind(objectElement)]);
       if (numEles > 1) typeName += std::to_string(numEles);

tools/clang/lib/Sema/SemaOverload.cpp

@@ -11020,7 +11020,7 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
     SemaRef.Diag(Fn->getLocStart(),
          diag::err_ovl_no_viable_function_in_call)
       << ULE->getName() << Fn->getSourceRange();
-    CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates, Args);
+    CandidateSet->NoteCandidates(SemaRef, OCD_AllCandidates, Args, StringRef(), ULE->getLocStart()); // HLSL Change - add loc
     break;
   }
 

tools/clang/test/DXILValidation/atomics.hlsl

@@ -0,0 +1,55 @@
+// Validation template including valid operations and invalid destinations
+// that the validator test will swap in to ensure that the validator produces
+// appropriate errors.
+
+// Valid resources for atomics to create valid output that will later be manipulated to test the validator
+RWStructuredBuffer<uint> rw_structbuf;
+RWBuffer<uint> rw_buf;
+RWTexture1D<uint> rw_tex;
+
+// SRVs to plug into atomics
+StructuredBuffer<uint> ro_structbuf;
+Buffer<uint> ro_buf;
+Texture1D<uint> ro_tex;
+
+const groupshared uint cgs_var = 0;
+const groupshared uint cgs_arr[3] = {0, 0, 0};
+
+groupshared uint gs_var;
+
+RWStructuredBuffer<uint> output; // just something to keep the variables alive
+
+cbuffer CB {
+  uint cb_var;
+}
+uint cb_gvar;
+
+#if __SHADER_TARGET_STAGE == __SHADER_STAGE_LIBRARY
+void init(out uint i); // To force an alloca pointer to use with atomic op
+#else
+void init(out uint i) {i = 0;}
+#endif
+
+[numthreads(1,1,1)]
+void main(uint ix : SV_GroupIndex) {
+
+  uint res;
+  init(res);
+
+  // Token usages of the invalid resources and variables so they are available in the output
+  res += cb_var + cb_gvar + cgs_var + ro_structbuf[ix] + ro_buf[ix] + ro_tex[ix] + cgs_arr[ix];
+
+  InterlockedAdd(rw_structbuf[ix], 1);
+  InterlockedCompareStore(rw_structbuf[ix], 1, 2);
+
+  InterlockedAdd(rw_buf[ix], 1);
+  InterlockedCompareStore(rw_buf[ix], 1, 2);
+
+  InterlockedAdd(rw_tex[ix], 1);
+  InterlockedCompareStore(rw_tex[ix], 1, 2);
+
+  InterlockedAdd(gs_var, 1);
+  InterlockedCompareStore(gs_var, 1, 2);
+
+  output[ix] = res;
+}

tools/clang/test/HLSL/atomics-on-bitfields.hlsl

@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -HV 2021 -fsyntax-only -ffreestanding -verify %s
+
+// Ensure that atomic operations fail when used with bitfields
+// Use structured and typed buffers as the resources that can use structs
+// and also both binary op and exchange atomic ops as either difference
+// can cause the compiler to take different paths
+
+struct TexCoords {
+  uint s : 8;
+  uint t : 8;
+  uint r : 8;
+  uint q : 8;
+};
+
+RWBuffer<TexCoords> buf;
+RWStructuredBuffer<TexCoords> str;
+groupshared TexCoords gs;
+
+[numthreads(8,8,1)]
+void main( uint2 tid : SV_DispatchThreadID) {
+
+  InterlockedOr(buf[tid.y].q, 2);                           /* expected-error {{no matching function for call to 'InterlockedOr'}} expected-note {{candidate function not viable: no known conversion from 'uint' to 'unsigned int &' for 1st argument}} expected-note {{candidate function not viable: no known conversion from 'uint' to 'unsigned long long &' for 1st argument}} */
+  InterlockedCompareStore(buf[tid.y].q, 3, 1);              /* expected-error {{no matching function for call to 'InterlockedCompareStore'}} expected-note {{candidate function not viable: no known conversion from 'uint' to 'unsigned int &' for 1st argument}} expected-note {{candidate function not viable: no known conversion from 'uint' to 'unsigned long long &' for 1st argument}} */
+
+  InterlockedOr(str[tid.y].q, 2);                           /* expected-error {{no matching function for call to 'InterlockedOr'}} expected-note {{candidate function not viable: no known conversion from 'uint' to 'unsigned int' for 1st argument}} expected-note {{candidate function not viable: no known conversion from 'uint' to 'unsigned long long' for 1st argument}} */
+  InterlockedCompareStore(str[tid.y].q, 3, 1);              /* expected-error {{no matching function for call to 'InterlockedCompareStore'}} expected-note {{candidate function not viable: no known conversion from 'uint' to 'unsigned int' for 1st argument}} expected-note {{candidate function not viable: no known conversion from 'uint' to 'unsigned long long' for 1st argument}} */
+
+  InterlockedOr(gs.q, 2);                                   /* expected-error {{no matching function for call to 'InterlockedOr'}} expected-note {{candidate function not viable: 1st argument ('__attribute__((address_space(3))) uint') is in address space 3, but parameter must be in address space 0}} expected-note {{candidate function not viable: no known conversion from '__attribute__((address_space(3))) uint' to 'unsigned long long' for 1st argument}} */
+  InterlockedCompareStore(gs.q, 3, 1);                      /* expected-error {{no matching function for call to 'InterlockedCompareStore'}} expected-note {{candidate function not viable: 1st argument ('__attribute__((address_space(3))) uint') is in address space 3, but parameter must be in address space 0}} expected-note {{candidate function not viable: no known conversion from '__attribute__((address_space(3))) uint' to 'unsigned long long' for 1st argument}} */
+}

tools/clang/test/HLSL/wave.hlsl

@@ -14,7 +14,7 @@ float4 main() : SV_Target {
     // Divergent, single thread executing here.
   }
   uint a = WaveGetLaneIndex();
-  
+
   if (WaveGetLaneCount() == 0) {
     // Unlikely!
   }
@@ -41,12 +41,9 @@ float4 main() : SV_Target {
   f3 = WaveActiveSum(f3);
   f3 = WaveActiveProduct(f3);
   // WaveActiveBit* with an invalid signature suggests the use of WaveActiveBallot instead.
-  // expected-note@? {{candidate function}}
-  // expected-note@? {{candidate function}}
-  // expected-note@? {{candidate function}}
-  WaveActiveBitAnd(f1); // expected-error {{no matching function for call to 'WaveActiveBitAnd'}}
-  WaveActiveBitOr(f1);  // expected-error {{no matching function for call to 'WaveActiveBitOr'}}
-  WaveActiveBitXor(f1); // expected-error {{no matching function for call to 'WaveActiveBitXor'}}
+  WaveActiveBitAnd(f1); // expected-error {{no matching function for call to 'WaveActiveBitAnd'}} expected-note {{candidate function not viable: no known conversion from 'float' to 'unsigned int' for 1st argument}}
+  WaveActiveBitOr(f1);  // expected-error {{no matching function for call to 'WaveActiveBitOr'}} expected-note {{candidate function not viable: no known conversion from 'float' to 'unsigned int' for 1st argument}}
+  WaveActiveBitXor(f1); // expected-error {{no matching function for call to 'WaveActiveBitXor'}} expected-note {{candidate function not viable: no known conversion from 'float' to 'unsigned int' for 1st argument}}
   u3 = WaveActiveBitAnd(u3);
   u3 = WaveActiveBitOr(u3);
   u3 = WaveActiveBitXor(u3);

tools/clang/test/HLSL/write-const-arrays.hlsl

@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -verify %s
+
+// Array subscripts lost their qualifiers including const due to taking a different
+// path for HLSL on account of having no array to ptr decay
+// This test verifies that subscripts of constant and cbuf arrays can't be assigned
+// or otherwise altered by a few mechanisms
+
+/* Expected note with no locations (implicit built-in):
+  expected-note@? {{function 'operator[]<const unsigned int &>' which returns const-qualified type 'const unsigned int &' declared here}}
+*/
+
+StructuredBuffer<uint> g_robuf;
+Texture1D<uint> g_tex;
+uint g_cbuf[4];
+
+const groupshared uint gs_val[4];
+
+[NumThreads(1, 1, 1)]
+void main() {
+  const uint local[4];
+
+  // Assigning using assignment operator
+  local[0] = 0;                                             /* expected-error {{read-only variable is not assignable}} */
+  gs_val[0] = 0;                                            /* expected-error {{read-only variable is not assignable}} */
+  g_cbuf[0] = 0;                                            /* expected-error {{read-only variable is not assignable}} */
+  g_robuf[0] = 0;                                           /* expected-error {{cannot assign to return value because function 'operator[]<const unsigned int &>' returns a const value}} */
+
+  // Assigning using out param of builtin function
+  double d = 1.0;
+  asuint(d, local[0], local[1]);                          /* expected-error {{no matching function for call to 'asuint'}} expected-note {{candidate function not viable: 2nd argument ('const uint') would lose const qualifier}} */
+  asuint(d, gs_val[0], gs_val[1]);                        /* expected-error {{no matching function for call to 'asuint'}} expected-note {{candidate function not viable: 2nd argument ('const uint') would lose const qualifier}} */
+  asuint(d, g_cbuf[0], g_cbuf[1]);                        /* expected-error {{no matching function for call to 'asuint'}} expected-note {{candidate function not viable: 2nd argument ('const uint') would lose const qualifier}} */
+  asuint(d, g_robuf[0], g_robuf[1]);                      /* expected-error {{no matching function for call to 'asuint'}} expected-note {{candidate function not viable: 2nd argument ('const unsigned int') would lose const qualifier}} */
+
+
+  // Assigning using out param of method
+  g_tex.GetDimensions(local[0]);                            /* expected-error {{no matching member function for call to 'GetDimensions'}} expected-note {{candidate function template not viable: requires 3 arguments, but 1 was provided}} */
+  g_tex.GetDimensions(gs_val[0]);                           /* expected-error {{no matching member function for call to 'GetDimensions'}} expected-note {{candidate function template not viable: requires 3 arguments, but 1 was provided}} */
+  g_tex.GetDimensions(g_cbuf[0]);                           /* expected-error {{no matching member function for call to 'GetDimensions'}} expected-note {{candidate function template not viable: requires 3 arguments, but 1 was provided}} */
+  g_tex.GetDimensions(g_robuf[0]);                          /* expected-error {{no matching member function for call to 'GetDimensions'}} expected-note {{candidate function template not viable: requires 3 arguments, but 1 was provided}} */
+
+
+  // Assigning using dest param of atomics
+  // Distinct because of special handling of atomics dest param
+  InterlockedAdd(local[0], 1);                              /* expected-error {{no matching function for call to 'InterlockedAdd'}} expected-note {{candidate function not viable: 1st argument ('const uint') would lose const qualifier}} expected-note {{candidate function not viable: no known conversion from 'const uint' to 'unsigned long long &' for 1st argument}} */
+  InterlockedAdd(gs_val[0], 1);                             /* expected-error {{no matching function for call to 'InterlockedAdd'}} expected-note {{candidate function not viable: 1st argument ('const uint') would lose const qualifier}} expected-note {{candidate function not viable: no known conversion from 'const uint' to 'unsigned long long' for 1st argument}} */
+  InterlockedAdd(g_cbuf[0], 1);                             /* expected-error {{no matching function for call to 'InterlockedAdd'}} expected-note {{candidate function not viable: 1st argument ('const uint') would lose const qualifier}} expected-note {{candidate function not viable: no known conversion from 'const uint' to 'unsigned long long' for 1st argument}} */
+  InterlockedAdd(g_robuf[0], 1);                            /* expected-error {{no matching function for call to 'InterlockedAdd'}} expected-note {{candidate function not viable: 1st argument ('const unsigned int') would lose const qualifier}} expected-note {{candidate function not viable: no known conversion from 'const unsigned int' to 'unsigned long long' for 1st argument}} */
+
+}

tools/clang/test/HLSLFileCheck/hlsl/intrinsics/atomic/atomic_on_members.hlsl

@@ -0,0 +1,79 @@
+// RUN: %dxc -HV 2021 -T cs_6_0 -DOP=InterlockedOr           -DDEST=Str %s | FileCheck %s -check-prefix=CHKBIN
+// RUN: %dxc -HV 2021 -T cs_6_0 -DOP=InterlockedCompareStore -DDEST=Str %s | FileCheck %s -check-prefix=CHKXCH
+// RUN: %dxc -HV 2021 -T cs_6_0 -DOP=InterlockedOr           -DDEST=Gs  %s | FileCheck %s -check-prefix=CHKGSB
+// RUN: %dxc -HV 2021 -T cs_6_0 -DOP=InterlockedCompareStore -DDEST=Gs  %s | FileCheck %s -check-prefix=CHKGSX
+// RUN: %dxc -HV 2021 -T cs_6_0 -DOP=InterlockedOr           -DDEST=Buf %s | FileCheck %s -check-prefix=CHKERR
+// RUN: %dxc -HV 2021 -T cs_6_0 -DOP=InterlockedCompareStore -DDEST=Buf %s | FileCheck %s -check-prefix=CHKERR
+
+// Ensure that atomic operations fail when used with struct members of a typed resource
+// The only typed resource that can use structs is RWBuffer
+// Use a binary op and an exchange op because they use different code paths
+
+struct Simple {
+  uint i;
+  uint longEnding[1];
+};
+
+struct Complex {
+  Simple s;
+  uint theEnd;
+};
+
+struct TexCoords {
+  uint s, t, r, q;
+};
+
+#define _PASTE(pre, res) pre##res
+#define PASTE(pre, res) _PASTE(pre, res)
+
+RWBuffer<TexCoords> Buf;
+RWBuffer<Simple> simpBuf;
+RWBuffer<Complex> cplxBuf;
+
+RWStructuredBuffer<TexCoords> Str;
+RWStructuredBuffer<Simple> simpStr;
+RWStructuredBuffer<Complex> cplxStr;
+
+groupshared TexCoords Gs[1];
+groupshared Simple simpGs[1];
+groupshared Complex cplxGs[1];
+
+[numthreads(8,8,1)]
+void main( uint3 gtid : SV_GroupThreadID , uint gid : SV_GroupIndex)
+{
+  uint orig = 0;
+  uint a = gid;
+  uint b = gtid.x;
+  uint c = gtid.y;
+  uint d = gtid.z;
+
+  // CHKBIN: call i32 @dx.op.atomicBinOp.i32(i32 78
+  // CHKBIN: call i32 @dx.op.atomicBinOp.i32(i32 78
+  // CHKBIN: call i32 @dx.op.atomicBinOp.i32(i32 78
+  // CHKBIN: call i32 @dx.op.atomicBinOp.i32(i32 78
+
+  // CHKXCH: call i32 @dx.op.atomicCompareExchange.i32(i32 79,
+  // CHKXCH: call i32 @dx.op.atomicCompareExchange.i32(i32 79,
+  // CHKXCH: call i32 @dx.op.atomicCompareExchange.i32(i32 79,
+  // CHKXCH: call i32 @dx.op.atomicCompareExchange.i32(i32 79,
+
+  // CHKGSB: atomicrmw or i32 addrspace(3)
+  // CHKGSB: atomicrmw or i32 addrspace(3)
+  // CHKGSB: atomicrmw or i32 addrspace(3)
+  // CHKGSB: atomicrmw or i32 addrspace(3)
+
+  // CHKGSX: cmpxchg i32 addrspace(3)
+  // CHKGSX: cmpxchg i32 addrspace(3)
+  // CHKGSX: cmpxchg i32 addrspace(3)
+  // CHKGSX: cmpxchg i32 addrspace(3)
+
+  // CHKERR: error: Typed resources used in atomic operations must have a scalar element type.
+  // CHKERR: error: Typed resources used in atomic operations must have a scalar element type.
+  // CHKERR: error: Typed resources used in atomic operations must have a scalar element type.
+  // CHKERR: error: Typed resources used in atomic operations must have a scalar element type.
+
+  OP( PASTE( ,DEST)[a].q, 2, orig );
+  OP( PASTE(simp, DEST)[a].i, 2, orig );
+  OP( PASTE(cplx, DEST)[a].s.i, 2, orig );
+  OP( PASTE(cplx, DEST)[a].s.longEnding[d].x, 2, orig );
+}

tools/clang/test/HLSLFileCheck/hlsl/intrinsics/atomic/atomic_restypes.hlsl

@@ -0,0 +1,128 @@
+// RUN: %dxc -T cs_6_0 -DDEST=ro_structbuf -DIDX=[0] %s | %FileCheck %s -check-prefixes=CHKRES,CHECK
+// RUN: %dxc -T cs_6_0 -DDEST=ro_buf -DIDX=[0] %s | %FileCheck %s -check-prefixes=CHKRES,CHECK
+// RUN: %dxc -T cs_6_0 -DDEST=ro_tex -DIDX=[0] %s | %FileCheck %s -check-prefixes=CHKRES,CHECK
+// RUN: %dxc -T cs_6_0 -DDEST=gs_var -DIDX=    %s | %FileCheck %s
+// RUN: %dxc -T cs_6_0 -DDEST=gs_arr -DIDX=[0] %s | %FileCheck %s
+// RUN: %dxc -T cs_6_0 -DDEST=cb_var -DIDX=    %s | %FileCheck %s
+// RUN: %dxc -T cs_6_0 -DDEST=cb_arr -DIDX=[0] %s | %FileCheck %s
+// RUN: %dxc -T cs_6_0 -DDEST=sc_var -DIDX=    %s | %FileCheck %s
+// RUN: %dxc -T cs_6_0 -DDEST=sc_arr -DIDX=[0] %s | %FileCheck %s
+// These are different because they aren't const, so are caught later
+// RUN: %dxc -T cs_6_0 -DDEST=loc_var -DIDX=    %s | %FileCheck %s -check-prefix=CHKLOC
+// RUN: %dxc -T cs_6_0 -DDEST=loc_arr -DIDX=[0] %s | %FileCheck %s -check-prefix=CHKLOC
+// RUN: %dxc -T cs_6_0 -DDEST=ix -DIDX=    %s | %FileCheck %s -check-prefix=CHKLOC
+
+
+// Test various Interlocked ops using different invalid destination memory types
+// The way the dest param of atomic ops is lowered is unique and missed a lot of
+// these invalid uses. There are a few points where the lowering branches depending
+// on the memory type, so this tries to cover all those branches:
+// groupshared, cbuffers, structbuffers, other resources, and other non-resources
+
+StructuredBuffer<uint> ro_structbuf;
+Buffer<uint> ro_buf;
+Texture1D<uint> ro_tex;
+
+const groupshared uint gs_var = 0;
+const groupshared uint gs_arr[4] = {0, 0, 0, 0};
+
+RWStructuredBuffer<float4> output; // just something to keep the variables live
+
+cbuffer CB {
+  uint cb_var;
+  uint cb_arr[4];
+}
+
+static const uint sc_var = 1;
+static const uint sc_arr[4] = {0,1,2,3};
+
+[numthreads(1,1,1)]
+void main(uint ix : SV_GroupIndex) {
+
+  uint loc_var;
+  uint loc_arr[4];
+
+  int val = 1;
+  uint comp = 1;
+  uint orig;
+
+  // add
+  // CHECK: error: no matching function for call to 'InterlockedAdd'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHECK: error: no matching function for call to 'InterlockedAdd'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedAdd(DEST IDX, val);
+  InterlockedAdd(DEST IDX, val, orig);
+
+  // min
+  // CHECK: error: no matching function for call to 'InterlockedMin'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHECK: error: no matching function for call to 'InterlockedMin'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedMin(DEST IDX, val);
+  InterlockedMin(DEST IDX, val, orig);
+
+  // max
+  // CHECK: error: no matching function for call to 'InterlockedMax'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHECK: error: no matching function for call to 'InterlockedMax'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedMax(DEST IDX, val);
+  InterlockedMax(DEST IDX, val, orig);
+
+  // and
+  // CHECK: error: no matching function for call to 'InterlockedAnd'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHECK: error: no matching function for call to 'InterlockedAnd'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedAnd(DEST IDX, val);
+  InterlockedAnd(DEST IDX, val, orig);
+
+  // or
+  // CHECK: error: no matching function for call to 'InterlockedOr'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHECK: error: no matching function for call to 'InterlockedOr'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedOr(DEST IDX, val);
+  InterlockedOr(DEST IDX, val, orig);
+
+  // xor
+  // CHECK: error: no matching function for call to 'InterlockedXor'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHECK: error: no matching function for call to 'InterlockedXor'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedXor(DEST IDX, val);
+  InterlockedXor(DEST IDX, val, orig);
+
+  // compareStore
+  // CHECK: error: no matching function for call to 'InterlockedCompareStore'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedCompareStore(DEST IDX, comp, val);
+
+  // exchange
+  // CHECK: error: no matching function for call to 'InterlockedExchange'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedExchange(DEST IDX, val, orig);
+
+  // compareExchange
+  // CHECK: error: no matching function for call to 'InterlockedCompareExchange'
+  // CHKRES: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+  // CHKLOC: error: Atomic operation targets must be groupshared or UAV
+  InterlockedCompareExchange(DEST IDX, comp, val, orig);
+
+  output[ix] = (float)DEST IDX;
+}

tools/clang/test/HLSLFileCheck/hlsl/intrinsics/atomic/atomic_restypes_lib.hlsl

@@ -0,0 +1,52 @@
+// RUN: %dxc -T lib_6_3 -DDEST=ro_structbuf %s | %FileCheck %s
+// RUN: %dxc -T lib_6_3 -DDEST=ro_structbuf %s | %FileCheck %s
+// RUN: %dxc -T lib_6_3 -DDEST=ro_buf %s | %FileCheck %s
+// RUN: %dxc -T lib_6_3 -DDEST=ro_buf %s | %FileCheck %s
+// RUN: %dxc -T lib_6_3 -DDEST=ro_tex %s | %FileCheck %s
+// RUN: %dxc -T lib_6_3 -DDEST=ro_tex %s | %FileCheck %s
+
+
+// Test that errors on atomic dest params will still fire when used in exported
+// functions in a library shader. Limits testing to one each of binop and xchg
+
+StructuredBuffer<uint> ro_structbuf;
+Buffer<uint> ro_buf;
+Texture1D<uint> ro_tex;
+
+RWStructuredBuffer<float4> output; // just something to keep the variables live
+
+// CHECK: error: no matching function for call to 'InterlockedAdd'
+// CHECK: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+// CHECK: error: no matching function for call to 'InterlockedAdd'
+// CHECK: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+// CHECK: error: no matching function for call to 'InterlockedAdd'
+// CHECK: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+export void AtomicAdd(StructuredBuffer<uint> res, uint val) {InterlockedAdd(res[0], val);}
+export void AtomicAdd(Buffer<uint> res, uint val) {InterlockedAdd(res[0], val);}
+export void AtomicAdd(Texture1D<uint> res, uint val) {InterlockedAdd(res[0], val);}
+
+// CHECK: error: no matching function for call to 'InterlockedCompareStore'
+// CHECK: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+// CHECK: error: no matching function for call to 'InterlockedCompareStore'
+// CHECK: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+// CHECK: error: no matching function for call to 'InterlockedCompareStore'
+// CHECK: note: candidate function not viable: 1st argument {{.*}} would lose const qualifier
+export void AtomicCompareStore(StructuredBuffer<uint> res, uint comp, uint val) {InterlockedCompareStore(res[0], comp, val);}
+export void AtomicCompareStore(Buffer<uint> res, uint comp, uint val) {InterlockedCompareStore(res[0], comp, val);}
+export void AtomicCompareStore(Texture1D<uint> res, uint comp, uint val) {InterlockedCompareStore(res[0], comp, val);}
+
+[numthreads(1,1,1)]
+void main(uint ix : SV_GroupIndex) {
+
+  int val = 1;
+  uint comp = 1;
+  uint orig;
+
+  // Add
+  AtomicAdd(DEST, val);
+
+  // CompareStore
+  AtomicCompareStore(DEST, comp, val);
+
+  output[ix] = (float)DEST[0];
+}

tools/clang/test/HLSLFileCheck/hlsl/intrinsics/atomic/atomic_structuredbuf_i64.hlsl

@@ -6,7 +6,7 @@
 struct simple {
   bool thisVariableIsFalse;
   uint64_t i;
-  float3x1 longEnding[4];
+  uint64_t3x1 longEnding[4];
 };
 
 struct complex {
@@ -22,53 +22,61 @@ RWStructuredBuffer<simple[3]> simpArrBuf;
 RWStructuredBuffer<complex> cplxBuf;
 RWStructuredBuffer<complex[3]> cplxArrBuf;
 
-void main( uint a : A, uint b: B, uint c :C) : SV_Target
+void main( uint a : A, uint b: B, uint c :C, uint d :D) : SV_Target
 {
   int64_t liv = a + b;
   int64_t liv2 = 0, liv3 = 0;
+  uint64_t loc_arr[4];
 
   // CHECK: call i64 @dx.op.atomicBinOp.i64
   // CHECK: call i64 @dx.op.atomicBinOp.i64
   // CHECK: call i64 @dx.op.atomicBinOp.i64
   // CHECK: call i64 @dx.op.atomicBinOp.i64
   // CHECK: call i64 @dx.op.atomicBinOp.i64
+  // CHECK: call i64 @dx.op.atomicBinOp.i64
   InterlockedAdd( simpBuf[a].i, liv );
   InterlockedAdd( simpArrBuf[a][b].i, liv );
   InterlockedAdd( cplxBuf[a].i, liv );
   InterlockedAdd( cplxBuf[a].s.i, liv );
   InterlockedAdd( cplxBuf[a].ss[b].i, liv );
+  InterlockedAdd( cplxBuf[a].ss[b].longEnding[c][d].x, liv);
 
   // CHECK: call i64 @dx.op.atomicBinOp.i64
   // CHECK: call i64 @dx.op.atomicBinOp.i64
   // CHECK: call i64 @dx.op.atomicBinOp.i64
   // CHECK: call i64 @dx.op.atomicBinOp.i64
   // CHECK: call i64 @dx.op.atomicBinOp.i64
+  // CHECK: call i64 @dx.op.atomicBinOp.i64
   InterlockedExchange( simpBuf[a].i, liv, liv2 );
   InterlockedExchange( simpArrBuf[a][b].i, liv2, liv );
   InterlockedExchange( cplxBuf[a].i, liv, liv2 );
   InterlockedExchange( cplxBuf[a].s.i, liv2, liv );
   InterlockedExchange( cplxBuf[a].ss[b].i, liv, liv2 );
+  InterlockedExchange( cplxBuf[a].ss[b].longEnding[c][d].x, liv, liv2);
 
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
+  // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   InterlockedCompareStore( simpBuf[a].i, liv, liv2 );
   InterlockedCompareStore( simpArrBuf[a][b].i, liv2, liv );
   InterlockedCompareStore( cplxBuf[a].i, liv, liv2 );
   InterlockedCompareStore( cplxBuf[a].s.i, liv2, liv );
   InterlockedCompareStore( cplxBuf[a].ss[b].i, liv, liv2 );
+  InterlockedCompareStore( cplxBuf[a].ss[b].longEnding[c][d].x, liv2, liv);
 
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   // CHECK: call i64 @dx.op.atomicCompareExchange.i64
+  // CHECK: call i64 @dx.op.atomicCompareExchange.i64
   InterlockedCompareExchange( simpBuf[a].i, liv, liv2, liv3 );
   InterlockedCompareExchange( simpArrBuf[a][b].i, liv2, liv3, liv );
   InterlockedCompareExchange( cplxBuf[a].i, liv3, liv2, liv );
   InterlockedCompareExchange( cplxBuf[a].s.i, liv2, liv, liv3 );
   InterlockedCompareExchange( cplxBuf[a].ss[b].i, liv2, liv3, liv );
-
+  InterlockedCompareExchange( cplxBuf[a].ss[b].longEnding[c][d].x, liv3, liv, liv2 );
 }

tools/clang/test/HLSLFileCheck/hlsl/operators/swizzle/swizzleAtomic2.hlsl

@@ -1,13 +1,26 @@
 // RUN: %dxc -E main -T cs_6_0 %s | FileCheck %s
 
-// CHECK: Typed resources used in atomic operations must have a scalar element type
+// Check that attempting to use atomic ops on swizzled typed resource members will fail
+// Use representatives of atomic binops and cmpexchange type atomic operations.
+// Include both typed buffer and texture resources
 
 RWBuffer<uint4> bufA;
+RWTexture1D<uint4> texA;
 
 [numthreads(8,8,1)]
 void main( uint2 tid : SV_DispatchThreadID, uint2 gid : SV_GroupID, uint2 gtid : SV_GroupThreadID, uint gidx : SV_GroupIndex )
 {
+    // CHECK: Typed resources used in atomic operations must have a scalar element type
+    // CHECK: Typed resources used in atomic operations must have a scalar element type
     bufA[tid.x] = gid.x;
     bufA[tid.y].z = gid.y;
     InterlockedOr(bufA[tid.y].y, 2);
- }
+    InterlockedCompareStore(bufA[tid.y].x, 3, 1);
+
+    // CHECK: Typed resources used in atomic operations must have a scalar element type
+    // CHECK: Typed resources used in atomic operations must have a scalar element type
+    texA[tid.x] = gid.x;
+    texA[tid.y].z = gid.y;
+    InterlockedOr(texA[tid.y].y, 2);
+    InterlockedCompareStore(texA[tid.y].x, 3, 1);
+}

tools/clang/unittests/HLSL/ValidationTest.cpp

@@ -300,6 +300,9 @@ public:
   TEST_METHOD(ValidateVersionNotAllowed)
   TEST_METHOD(CreateHandleNotAllowedSM66)
 
+  TEST_METHOD(AtomicsConsts)
+  TEST_METHOD(AtomicsInvalidDests)
+
   dxc::DxcDllSupport m_dllSupport;
   VersionSupportInfo m_ver;
 
@@ -3902,3 +3905,121 @@ TEST_F(ValidationTest, CreateHandleNotAllowedSM66) {
     "opcode 'CreateHandle' should only be used in 'non-library targets'",
     true);
 }
+
+// Check for validation errors for various const dests to atomics
+TEST_F(ValidationTest, AtomicsConsts) {
+  if (m_ver.SkipDxilVersion(1, 7)) return;
+  std::vector<LPCWSTR> pArguments = { L"-HV", L"2021", L"-Zi"};
+
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %rw_structbuf_UAV_structbuf"},
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %ro_structbuf_texture_structbuf"},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %rw_structbuf_UAV_structbuf"},
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %ro_structbuf_texture_structbuf"},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %rw_buf_UAV_buf"},
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %ro_buf_texture_buf"},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %rw_buf_UAV_buf"},
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %ro_buf_texture_buf"},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %rw_tex_UAV_1d"},
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %ro_tex_texture_1d"},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %rw_tex_UAV_1d"},
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %ro_tex_texture_1d"},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %rw_buf_UAV_buf"},
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %CB_cbuffer"},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %rw_buf_UAV_buf"},
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %CB_cbuffer"},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %rw_buf_UAV_buf"},
+    {"call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle %\"$Globals_cbuffer\""},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %rw_buf_UAV_buf"},
+    {"call i32 @dx.op.atomicCompareExchange.i32(i32 79, %dx.types.Handle %\"$Globals_cbuffer\""},
+    "Non-UAV destination to atomic intrinsic.",
+    false);
+
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"atomicrmw add i32 addrspace(3)* @\"\\01?gs_var@@3IA\""},
+    {"atomicrmw add i32 addrspace(3)* @\"\\01?cgs_var@@3IB\""},
+    "Constant destination to atomic.",
+    false);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"cmpxchg i32 addrspace(3)* @\"\\01?gs_var@@3IA\""},
+    {"cmpxchg i32 addrspace(3)* @\"\\01?cgs_var@@3IB\""},
+    "Constant destination to atomic.",
+    false);
+
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"%([a-zA-Z0-9]+) = getelementptr \\[3 x i32\\], \\[3 x i32\\] addrspace\\(3\\)\\* @\"\\\\01\\?cgs_arr@@3QBIB\"([^\n]*)"},
+    {"%\\1 = getelementptr \\[3 x i32\\], \\[3 x i32\\] addrspace\\(3\\)\\* @\"\\\\01\\?cgs_arr@@3QBIB\"\\2\n"
+     "%dummy = atomicrmw add i32 addrspace\\(3\\)\\* %\\1, i32 1 seq_cst, !dbg !104 ; line:51 col:3"
+    },
+    "Constant destination to atomic.",
+    true);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "cs_6_0",
+    pArguments.data(), 3, nullptr, 0,
+    {"%([a-zA-Z0-9]+) = getelementptr \\[3 x i32\\], \\[3 x i32\\] addrspace\\(3\\)\\* @\"\\\\01\\?cgs_arr@@3QBIB\"([^\n]*)"},
+    {"%\\1 = getelementptr \\[3 x i32\\], \\[3 x i32\\] addrspace\\(3\\)\\* @\"\\\\01\\?cgs_arr@@3QBIB\"\\2\n"
+     "%dummy = cmpxchg i32 addrspace\\(3\\)\\* %\\1, i32 1, i32 2 seq_cst seq_cst, !dbg !105 ;"},
+    "Constant destination to atomic.",
+    true);
+}
+
+// Check validation error for non-groupshared dest
+TEST_F(ValidationTest, AtomicsInvalidDests) {
+  if (m_ver.SkipDxilVersion(1, 7)) return;
+  std::vector<LPCWSTR> pArguments = { L"-HV", L"2021", L"-Zi" };
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "lib_6_3",
+    pArguments.data(), 2, nullptr, 0,
+    {"atomicrmw add i32 addrspace(3)* @\"\\01?gs_var@@3IA\""},
+    {"atomicrmw add i32* %res"},
+    "Non-groupshared destination to atomic operation.",
+    false);
+  RewriteAssemblyCheckMsg(L"..\\DXILValidation\\atomics.hlsl", "lib_6_3",
+    pArguments.data(), 2, nullptr, 0,
+    {"cmpxchg i32 addrspace(3)* @\"\\01?gs_var@@3IA\""},
+    {"cmpxchg i32* %res"},
+    "Non-groupshared destination to atomic operation.",
+    false);
+
+}

tools/clang/unittests/HLSL/VerifierTest.cpp

@@ -97,6 +97,8 @@ public:
   TEST_METHOD(RunVectorOr)
   TEST_METHOD(RunArrayConstAssign)
   TEST_METHOD(RunInputPatchConst)
+  TEST_METHOD(RunWriteConstArrays)
+  TEST_METHOD(RunAtomicsOnBitfields)
 
   void CheckVerifies(const wchar_t* path) {
     WEX::TestExecution::SetVerifyOutput verifySettings(WEX::TestExecution::VerifyOutputSettings::LogOnlyFailures);
@@ -419,3 +421,11 @@ TEST_F(VerifierTest, RunArrayConstAssign) {
 TEST_F(VerifierTest, RunInputPatchConst) {
   CheckVerifiesHLSL(L"InputPatch-const.hlsl");
 }
+
+TEST_F(VerifierTest, RunWriteConstArrays) {
+  CheckVerifiesHLSL(L"write-const-arrays.hlsl");
+}
+
+TEST_F(VerifierTest, RunAtomicsOnBitfields) {
+  CheckVerifiesHLSL(L"atomics-on-bitfields.hlsl");
+}

utils/hct/VerifierHelper.py

@@ -112,6 +112,8 @@ VerifierTests = {
     'RunVectorSyntaxExactPrecision':             'vector-syntax-exact-precision.hlsl',
     'RunVectorSyntaxMix':                        'vector-syntax-mix.hlsl',
     'RunWave':                                   'wave.hlsl',
+    'RunWriteConstArrays':                       'write-const-arrays.hlsl',
+    'RunAtomicsOnBitfields':                     'atomics-on-bitfields.hlsl',
 }
 
 # The following test(s) do not work in fxc mode:
@@ -138,6 +140,7 @@ fxcExcludedTests = [
     'RunTemplateLiteralSubstitutionFailure',
     'RunVectorSyntaxExactPrecision',
     'RunWave',
+    'RunAtomicsOnBitfields',
 ]
 
 # rxRUN = re.compile(r'[ RUN      ] VerifierTest.(\w+)')	# gtest syntax

utils/hct/hctdb.py

@@ -2623,6 +2623,9 @@ class db_dxil(object):
         self.add_valrule("Instr.MultipleGetMeshPayload", "GetMeshPayload cannot be called multiple times.")
         self.add_valrule("Instr.NotOnceDispatchMesh", "DispatchMesh must be called exactly once in an Amplification shader.")
         self.add_valrule("Instr.NonDominatingDispatchMesh", "Non-Dominating DispatchMesh call.")
+        self.add_valrule("Instr.AtomicOpNonGroupshared", "Non-groupshared destination to atomic operation.")
+        self.add_valrule("Instr.AtomicIntrinNonUAV", "Non-UAV destination to atomic intrinsic.")
+        self.add_valrule("Instr.AtomicConst", "Constant destination to atomic.")
 
         # Some legacy rules:
         # - space is only supported for shader targets 5.1 and higher