Skip to content

Commit ca15868

Browse files
committed
Add validation for atomic operations
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 shader-slang#5989.
1 parent a7e475a commit ca15868

5 files changed

+110
-0
lines changed

source/slang/slang-diagnostic-defs.h

+6
Original file line numberDiff line numberDiff line change
@@ -2251,6 +2251,12 @@ DIAGNOSTIC(
22512251
multiSampledTextureDoesNotAllowWrites,
22522252
"cannot write to a multisampled texture with target '$0'.")
22532253

2254+
DIAGNOSTIC(
2255+
41403,
2256+
Error,
2257+
invalidAtomicDestinationPointer,
2258+
"cannot perform atomic operation because destination is neither groupshared nor from a device buffer.")
2259+
22542260
//
22552261
// 5xxxx - Target code generation.
22562262
//

source/slang/slang-emit.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -1322,6 +1322,8 @@ Result linkAndOptimizeIR(
13221322
byteAddressBufferOptions);
13231323
}
13241324

1325+
validateAtomicOperations(sink, irModule->getModuleInst());
1326+
13251327
// For CUDA targets only, we will need to turn operations
13261328
// the implicitly reference the "active mask" into ones
13271329
// that use (and pass around) an explicit mask instead.

source/slang/slang-ir-insts.h

+8
Original file line numberDiff line numberDiff line change
@@ -2415,6 +2415,7 @@ struct IRLoad : IRInst
24152415
IRUse ptr;
24162416
IR_LEAF_ISA(Load)
24172417

2418+
IRInst* getAddress() { return getOperand(0); }
24182419
IRInst* getPtr() { return ptr.get(); }
24192420
};
24202421

@@ -2491,6 +2492,13 @@ struct IRGetElementPtr : IRInst
24912492
IRInst* getIndex() { return getOperand(1); }
24922493
};
24932494

2495+
struct IRGetOffsetPtr : IRInst
2496+
{
2497+
IR_LEAF_ISA(GetOffsetPtr);
2498+
IRInst* getBase() { return getOperand(0); }
2499+
IRInst* getOffset() { return getOperand(1); }
2500+
};
2501+
24942502
struct IRRWStructuredBufferGetElementPtr : IRInst
24952503
{
24962504
IR_LEAF_ISA(RWStructuredBufferGetElementPtr);

source/slang/slang-ir-validate.cpp

+92
Original file line numberDiff line numberDiff line change
@@ -410,4 +410,96 @@ void validateIRModuleIfEnabled(CodeGenContext* codeGenContext, IRModule* module)
410410
validateIRModule(module, sink);
411411
}
412412

413+
// Returns whether 'dst' is a valid destination for atomic operations, meaning
414+
// it leads either to 'groupshared' or 'device buffer' memory.
415+
static bool isValidAtomicDest(IRInst* dst)
416+
{
417+
bool isGroupShared = as<IRGroupSharedRate>(dst->getRate());
418+
if (isGroupShared)
419+
return true;
420+
421+
if (as<IRRWStructuredBufferGetElementPtr>(dst))
422+
return true;
423+
if (as<IRImageSubscript>(dst))
424+
return true;
425+
426+
if (auto ptrType = as<IRPtrType>(dst->getDataType()))
427+
{
428+
switch (ptrType->getAddressSpace())
429+
{
430+
case AddressSpace::Global:
431+
case AddressSpace::GroupShared:
432+
case AddressSpace::StorageBuffer:
433+
case AddressSpace::UserPointer:
434+
return true;
435+
default:
436+
break;
437+
}
438+
}
439+
440+
if (as<IRGlobalParam>(dst))
441+
{
442+
switch (dst->getDataType()->getOp())
443+
{
444+
case kIROp_GLSLShaderStorageBufferType:
445+
case kIROp_TextureType:
446+
return true;
447+
default:
448+
return false;
449+
}
450+
}
451+
452+
if (auto param = as<IRParam>(dst))
453+
if (auto outType = as<IROutTypeBase>(param->getDataType()))
454+
if (outType->getAddressSpace() == AddressSpace::GroupShared)
455+
return true;
456+
if (auto getElementPtr = as<IRGetElementPtr>(dst))
457+
return isValidAtomicDest(getElementPtr->getBase());
458+
if (auto getOffsetPtr = as<IRGetOffsetPtr>(dst))
459+
return isValidAtomicDest(getOffsetPtr->getBase());
460+
if (auto fieldAddress = as<IRFieldAddress>(dst))
461+
return isValidAtomicDest(fieldAddress->getBase());
462+
463+
return false;
464+
}
465+
466+
void validateAtomicOperations(DiagnosticSink* sink, IRInst* inst)
467+
{
468+
// There may be unused functions containing violations after address space specialization.
469+
if (auto func = as<IRFunc>(inst))
470+
if(!(func->hasUses() || func->findDecoration<IREntryPointDecoration>()))
471+
return;
472+
473+
switch (inst->getOp())
474+
{
475+
case kIROp_AtomicLoad:
476+
case kIROp_AtomicStore:
477+
case kIROp_AtomicExchange:
478+
case kIROp_AtomicCompareExchange:
479+
case kIROp_AtomicAdd:
480+
case kIROp_AtomicSub:
481+
case kIROp_AtomicAnd:
482+
case kIROp_AtomicOr:
483+
case kIROp_AtomicXor:
484+
case kIROp_AtomicMin:
485+
case kIROp_AtomicMax:
486+
case kIROp_AtomicInc:
487+
case kIROp_AtomicDec:
488+
{
489+
IRInst* destinationPtr = inst->getOperand(0);
490+
if (!isValidAtomicDest(destinationPtr))
491+
sink->diagnose(inst->sourceLoc, Diagnostics::invalidAtomicDestinationPointer);
492+
}
493+
break;
494+
495+
default:
496+
break;
497+
}
498+
499+
for (auto child : inst->getModifiableChildren())
500+
{
501+
validateAtomicOperations(sink, child);
502+
}
503+
}
504+
413505
} // namespace Slang

source/slang/slang-ir-validate.h

+2
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,6 @@ void validateIRModuleIfEnabled(CodeGenContext* codeGenContext, IRModule* module)
3838
void disableIRValidationAtInsert();
3939
void enableIRValidationAtInsert();
4040

41+
void validateAtomicOperations(DiagnosticSink* sink, IRInst* inst);
42+
4143
} // namespace Slang

0 commit comments

Comments
 (0)