Skip to content

[SER] Validate HitObject accessors #7371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 25, 2025
Merged

[SER] Validate HitObject accessors #7371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b3602bd
[SER] Validate HitObject accessors
simoll Apr 23, 2025
9095a7b
Merge remote-tracking branch 'msft/main' into ser_dxvaccessors_patch
simoll Apr 23, 2025
d411bb1
Make Instr.ParamMultiple msg conform to standards
simoll Apr 23, 2025
a80958d
nfc: Make tests use a valid multiple-of-4 for hitObject_LoadLocalRoot…
simoll Apr 23, 2025
63126f6
Fix bounds check (was off by one)
simoll Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/DXIL.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3161,6 +3161,7 @@ INSTR.OPCODERESERVED Instructions must not refe
INSTR.OPCONST DXIL intrinsic requires an immediate constant operand
INSTR.OPCONSTRANGE Constant values must be in-range for operation.
INSTR.OPERANDRANGE DXIL intrinsic operand must be within defined range
INSTR.PARAMMULTIPLE Parameter must be a valid multiple
INSTR.PTRBITCAST Pointer type bitcast must be have same size.
INSTR.RESOURCECLASSFORLOAD load can only run on UAV/SRV resource.
INSTR.RESOURCECLASSFORSAMPLERGATHER sample, lod and gather should be on srv resource.
Expand Down
110 changes: 110 additions & 0 deletions lib/DxilValidation/DxilValidation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1644,6 +1644,46 @@ static unsigned getSemanticFlagValidMask(const ShaderModel *pSM) {
return static_cast<unsigned>(hlsl::DXIL::BarrierSemanticFlag::ValidMask);
}

StringRef GetOpCodeName(DXIL::OpCode OpCode) {
switch (OpCode) {
default:
DXASSERT(false, "Unexpected op code");
return "";
case DXIL::OpCode::HitObject_ObjectRayOrigin:
return "HitObject_ObjectRayOrigin";
case DXIL::OpCode::HitObject_WorldRayDirection:
return "HitObject_WorldRayDirection";
case DXIL::OpCode::HitObject_WorldRayOrigin:
return "HitObject_WorldRayOrigin";
case DXIL::OpCode::HitObject_ObjectRayDirection:
return "HitObject_ObjectRayDirection";
case DXIL::OpCode::HitObject_WorldToObject3x4:
return "HitObject_WorldToObject3x4";
case DXIL::OpCode::HitObject_ObjectToWorld3x4:
return "HitObject_ObjectToWorld3x4";
}
}

static void ValidateConstantRangeUnsigned(Value *Val, StringRef Name,
uint64_t LowerBound,
uint64_t UpperBound, CallInst *CI,
DXIL::OpCode OpCode,
ValidationContext &ValCtx) {
ConstantInt *C = dyn_cast<ConstantInt>(Val);
if (!C) {
ValCtx.EmitInstrFormatError(CI, ValidationRule::InstrOpConst,
{Name, GetOpCodeName(OpCode)});
return;
}
if (C->uge(UpperBound + 1U) || !C->uge(LowerBound)) {
std::string Range =
std::to_string(LowerBound) + "~" + std::to_string(UpperBound);
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrOperandRange,
{Name, Range, C->getValue().toString(10, false)});
}
}

static void ValidateDxilOperationCallInProfile(CallInst *CI,
DXIL::OpCode Opcode,
const ShaderModel *pSM,
Expand Down Expand Up @@ -1910,6 +1950,76 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
CI, ValidationRule::InstrMayReorderThreadUndefCoherenceHintParam);
} break;

case DXIL::OpCode::HitObject_LoadLocalRootTableConstant: {
Value *HitObject = CI->getArgOperand(1);
if (isa<UndefValue>(HitObject))
ValCtx.EmitInstrError(CI, ValidationRule::InstrUndefHitObject);
Value *Offset = CI->getArgOperand(2);
if (isa<UndefValue>(Offset))
ValCtx.EmitInstrError(CI, ValidationRule::InstrNoReadingUninitialized);
if (ConstantInt *COffset = dyn_cast<ConstantInt>(Offset)) {
if (COffset->getLimitedValue() % 4 != 0)
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrParamMultiple,
{"offset", "4", COffset->getValue().toString(10, false)});
}
break;
}
case DXIL::OpCode::HitObject_SetShaderTableIndex: {
Value *HitObject = CI->getArgOperand(1);
if (isa<UndefValue>(HitObject))
ValCtx.EmitInstrError(CI, ValidationRule::InstrUndefHitObject);
Value *RecordIndex = CI->getArgOperand(2);
if (isa<UndefValue>(RecordIndex))
ValCtx.EmitInstrError(CI, ValidationRule::InstrNoReadingUninitialized);
break;
}

// Shader Execution Reordering - scalar getters
case DXIL::OpCode::HitObject_GeometryIndex:
case DXIL::OpCode::HitObject_HitKind:
case DXIL::OpCode::HitObject_InstanceID:
case DXIL::OpCode::HitObject_InstanceIndex:
case DXIL::OpCode::HitObject_IsHit:
case DXIL::OpCode::HitObject_IsMiss:
case DXIL::OpCode::HitObject_IsNop:
case DXIL::OpCode::HitObject_PrimitiveIndex:
case DXIL::OpCode::HitObject_RayFlags:
case DXIL::OpCode::HitObject_RayTCurrent:
case DXIL::OpCode::HitObject_RayTMin:
case DXIL::OpCode::HitObject_ShaderTableIndex: {
Value *HitObject = CI->getArgOperand(1);
if (isa<UndefValue>(HitObject))
ValCtx.EmitInstrError(CI, ValidationRule::InstrUndefHitObject);
break;
}

// Shader Execution Reordering - vector getters
case DXIL::OpCode::HitObject_ObjectRayDirection:
case DXIL::OpCode::HitObject_ObjectRayOrigin:
case DXIL::OpCode::HitObject_WorldRayDirection:
case DXIL::OpCode::HitObject_WorldRayOrigin: {
Value *HitObject = CI->getArgOperand(1);
if (isa<UndefValue>(HitObject))
ValCtx.EmitInstrError(CI, ValidationRule::InstrUndefHitObject);
Value *Col = CI->getArgOperand(2);
ValidateConstantRangeUnsigned(Col, "component", 0, 2, CI, Opcode, ValCtx);
break;
}

// Shader Execution Reordering - matrix getters
case DXIL::OpCode::HitObject_WorldToObject3x4:
case DXIL::OpCode::HitObject_ObjectToWorld3x4: {
Value *HitObject = CI->getArgOperand(1);
if (isa<UndefValue>(HitObject))
ValCtx.EmitInstrError(CI, ValidationRule::InstrUndefHitObject);
Value *Row = CI->getArgOperand(2);
ValidateConstantRangeUnsigned(Row, "row", 0, 2, CI, Opcode, ValCtx);
Value *Col = CI->getArgOperand(3);
ValidateConstantRangeUnsigned(Col, "column", 0, 3, CI, Opcode, ValCtx);
break;
}

case DXIL::OpCode::AtomicBinOp:
case DXIL::OpCode::AtomicCompareExchange: {
Type *pOverloadType = OP::GetOverloadType(Opcode, CI->getCalledFunction());
Expand Down
4 changes: 2 additions & 2 deletions tools/clang/test/CodeGenDXIL/hlsl/objects/HitObject/hitobject_accessors.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// DXIL-DAG: %{{[^ ]+}} = call i32 @dx.op.hitObject_StateScalar.i32(i32 283, %dx.types.HitObject %[[HIT]]) ; HitObject_InstanceID(hitObject)
// DXIL-DAG: %{{[^ ]+}} = call i32 @dx.op.hitObject_StateScalar.i32(i32 284, %dx.types.HitObject %[[HIT]]) ; HitObject_PrimitiveIndex(hitObject)
// DXIL-DAG: %{{[^ ]+}} = call i32 @dx.op.hitObject_StateScalar.i32(i32 286, %dx.types.HitObject %[[HIT]]) ; HitObject_ShaderTableIndex(hitObject)
// DXIL-DAG: %{{[^ ]+}} = call i32 @dx.op.hitObject_LoadLocalRootTableConstant(i32 288, %dx.types.HitObject %[[HIT]], i32 42) ; HitObject_LoadLocalRootTableConstant(hitObject,offset)
// DXIL-DAG: %{{[^ ]+}} = call i32 @dx.op.hitObject_LoadLocalRootTableConstant(i32 288, %dx.types.HitObject %[[HIT]], i32 40) ; HitObject_LoadLocalRootTableConstant(hitObject,offset)
// DXIL-DAG: %{{[^ ]+}} = call float @dx.op.hitObject_StateVector.f32(i32 277, %dx.types.HitObject %[[HIT]], i32 0) ; HitObject_ObjectRayOrigin(hitObject,component)
// DXIL-DAG: %{{[^ ]+}} = call float @dx.op.hitObject_StateVector.f32(i32 277, %dx.types.HitObject %[[HIT]], i32 1) ; HitObject_ObjectRayOrigin(hitObject,component)
// DXIL-DAG: %{{[^ ]+}} = call float @dx.op.hitObject_StateVector.f32(i32 277, %dx.types.HitObject %[[HIT]], i32 2) ; HitObject_ObjectRayOrigin(hitObject,component)
Expand Down Expand Up @@ -88,7 +88,7 @@ void main() {
isum += hit.GetInstanceID();
isum += hit.GetPrimitiveIndex();
isum += hit.GetShaderTableIndex();
isum += hit.LoadLocalRootTableConstant(42);
isum += hit.LoadLocalRootTableConstant(40);

// float3 accessors
vsum += hit.GetWorldRayOrigin();
Expand Down
Loading