Skip to content

Commit

Permalink
Add validation for atomic operations
Browse files Browse the repository at this point in the history
Diagnose an error if the destination of the atomic operation is not appropriate, where
appropriate means it's either:
- 'groupshared'
- from a device buffer

This closes #5989.
  • Loading branch information
aleino-nv committed Jan 15, 2025
1 parent 730ce98 commit ffeaabc
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 0 deletions.
6 changes: 6 additions & 0 deletions source/slang/slang-diagnostic-defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -2241,6 +2241,12 @@ DIAGNOSTIC(
multiSampledTextureDoesNotAllowWrites,
"cannot write to a multisampled texture with target '$0'.")

DIAGNOSTIC(
41403,
Error,
invalidAtomicDestinationPointer,
"cannot perform atomic operation because destination is neither groupshared nor from a device buffer.")

//
// 5xxxx - Target code generation.
//
Expand Down
3 changes: 3 additions & 0 deletions source/slang/slang-emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "slang-ir-insts.h"
#include "slang-ir-layout.h"
#include "slang-ir-legalize-array-return-type.h"
#include "slang-ir-legalize-atomic-operations.h"
#include "slang-ir-legalize-image-subscript.h"
#include "slang-ir-legalize-mesh-outputs.h"
#include "slang-ir-legalize-uniform-buffer-load.h"
Expand Down Expand Up @@ -1301,6 +1302,8 @@ Result linkAndOptimizeIR(
byteAddressBufferOptions);
}

legalizeAtomicOperations(sink, irModule);

// For CUDA targets only, we will need to turn operations
// the implicitly reference the "active mask" into ones
// that use (and pass around) an explicit mask instead.
Expand Down
74 changes: 74 additions & 0 deletions source/slang/slang-ir-legalize-atomic-operations.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#include "slang-ir-legalize-atomic-operations.h"

#include "slang-ir-insts.h"

namespace Slang
{

static bool isFromDeviceBuffer(IRInst* dst)
{
if (as<IRRWStructuredBufferGetElementPtr>(dst))
return true;
if (as<IRGlobalParam>(dst))
{
switch (dst->getDataType()->getOp())
{
case kIROp_GLSLShaderStorageBufferType:
return true;
default:
return false;
}
}

if (auto getElementPtr = as<IRGetElementPtr>(dst))
return isFromDeviceBuffer(getElementPtr->getBase());
if (auto fieldAddress = as<IRFieldAddress>(dst))
return isFromDeviceBuffer(fieldAddress->getBase());

return false;
}

static void validateAtomicOperations(DiagnosticSink* sink, IRInst* inst)
{
switch (inst->getOp())
{
case kIROp_AtomicLoad:
case kIROp_AtomicStore:
case kIROp_AtomicExchange:
case kIROp_AtomicCompareExchange:
case kIROp_AtomicAdd:
case kIROp_AtomicSub:
case kIROp_AtomicAnd:
case kIROp_AtomicOr:
case kIROp_AtomicXor:
case kIROp_AtomicMin:
case kIROp_AtomicMax:
case kIROp_AtomicInc:
case kIROp_AtomicDec:
{
IRInst* destinationPtr = inst->getOperand(0);
IRRate* destinationPtrRate = destinationPtr->getRate();
bool isGroupShared = as<IRGroupSharedRate>(destinationPtrRate);
bool desinationPtrValid = isGroupShared || isFromDeviceBuffer(destinationPtr);
if (!desinationPtrValid)
{
sink->diagnose(inst->sourceLoc, Diagnostics::invalidAtomicDestinationPointer);
}
}
break;

default:
break;
}

for (auto child : inst->getModifiableChildren())
{
validateAtomicOperations(sink, child);
}
}

void legalizeAtomicOperations(DiagnosticSink * sink, IRModule* module)
{
validateAtomicOperations(sink, module->getModuleInst());
}
} // namespace Slang
9 changes: 9 additions & 0 deletions source/slang/slang-ir-legalize-atomic-operations.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#pragma once

namespace Slang
{
class DiagnosticSink;
struct IRModule;

void legalizeAtomicOperations(DiagnosticSink * sink, IRModule* module);
} // namespace Slang

0 comments on commit ffeaabc

Please sign in to comment.