Skip to content

Commit 708e1bb

Browse files
csyongheCopilot
andauthored
Use CopyLogical to further defer storage-to-logical translation when emitting SPIRV. (#8819)
In #8526, we implemented an optimization that defers translation to end of access chains from strorage type to logical type until a leaf logical value is needed. Upon seeing: ``` mutablePtr : ptr<SomeStorageType> = ... t = translateStorageToLogical(mutablePtr); result = load(t) ``` We will create a local var to hold the result of the load, but defer the conversion logic from storage type to logical type past the load, so we translate the code into: ``` var tmp; v = load(mutablePtr) store(tmp, v); result = translateStorageToLogicalDeref(tmp) ... ``` So that after this translation, we can continue pushing the `translateStorageToLogicalDeref` to the users of the `result`. Unfortunately, this translation breaks a vulkan-spirv validation rule: because `tmp` will have `SomeStorageType`, which is a type that has explicit layout, it cannot be used to declare a local variable in the `Function` address space. Due to this, we rolled back this optimization and stop deferring storage-to-logical conversion past the loads from mutable locations in #8752. In this PR, we re-enables the deferring logic, by translating into the following code instead: ``` var tmp : Ptr<SomeStorageType_logical>; // load from `mutablePtr`, convert it from SomeStorageType // to SomeStorageType_logical, and write to `tmp`. copyLogical(tmp, mutablePtr) result = translateStorageToLogicalDeref(tmp) ``` Where `SomeStorageType_logical` is a clone of `SomeStorageType` that is the same in all aspects, except that `SomeStorageType_logical` does not have a `PhysicalType` decoration and won't be emitted with explicit layouts when emitting SPIRV. This means that `SomeStorageType_logical` is still a "storage type" in Slang's terminology (since `bool` are represented as `uint32_t`, `float4x4` is still represented in a `_MatrixStorage_float_4x4` struct), but is a "logical" type in SPIRV terminology because it does not have explicit offsets or array strides. The `copyLogical` inst can be implemented by the `OpCopyLogical` in SPIRV 1.4. When emitting code for older SPIRV versions, we will run a dedicated pass to convert the `copyLogical` into recursive field-by-field/element-by-element copies. Using SPIRV's `CopyLogical` allow us to generate cleaner code that has higher chance of being optimized out by the driver. Part of #8652. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 8f7c1b0 commit 708e1bb

15 files changed

Lines changed: 563 additions & 72 deletions

source/slang/slang-emit-spirv.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4196,6 +4196,9 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
41964196
case kIROp_BitCast:
41974197
result = emitOpBitcast(parent, inst, inst->getDataType(), inst->getOperand(0));
41984198
break;
4199+
case kIROp_CopyLogical:
4200+
result = emitCopyLogical(parent, as<IRCopyLogical>(inst));
4201+
break;
41994202
case kIROp_BitfieldExtract:
42004203
result = emitBitfieldExtract(parent, inst);
42014204
break;
@@ -6739,6 +6742,11 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
67396742
break;
67406743
case kIROp_Loop:
67416744
argStartIndex = 3;
6745+
// The use from a loop inst is a true source of control flow
6746+
// only if the use is the loopInst's target block operand.
6747+
// A use from loop's continue block shouldn't count as an incoming block.
6748+
if (use != as<IRLoop>(branchInst)->getTargetBlockUse())
6749+
continue;
67426750
break;
67436751
default:
67446752
// A phi argument can only come from an unconditional branch inst.
@@ -7933,6 +7941,25 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
79337941
inst->getOperand(0));
79347942
}
79357943

7944+
SpvInst* emitCopyLogical(SpvInstParent* parent, IRCopyLogical* inst)
7945+
{
7946+
IRBuilder builder(inst);
7947+
builder.setInsertBefore(inst);
7948+
auto dstValType = tryGetPointedToType(&builder, inst->getPtr()->getDataType());
7949+
auto srcValType = tryGetPointedToType(&builder, inst->getVal()->getDataType());
7950+
ShortList<IRInst*> attribs;
7951+
for (auto attrib : inst->getAllAttrs())
7952+
{
7953+
attribs.add(attrib);
7954+
}
7955+
auto slangIRLoad = as<IRLoad>(
7956+
builder.emitLoad(srcValType, inst->getVal(), attribs.getArrayView().arrayView));
7957+
auto srcVal = emitLoad(parent, slangIRLoad);
7958+
auto convertedVal =
7959+
emitInst(parent, nullptr, SpvOpCopyLogical, dstValType, kResultID, srcVal);
7960+
return emitOpStore(parent, nullptr, inst->getPtr(), convertedVal);
7961+
}
7962+
79367963
SpvInst* emitBitfieldExtract(SpvInstParent* parent, IRInst* inst)
79377964
{
79387965
auto dataType = inst->getDataType();

source/slang/slang-ir-insts-stable-names.lua

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ return {
223223
["field"] = 219,
224224
["var"] = 220,
225225
["load"] = 221,
226-
["store"] = 222,
226+
["StoreBase.store"] = 222,
227227
["AtomicOperation.atomicLoad"] = 223,
228228
["AtomicOperation.atomicStore"] = 224,
229229
["AtomicOperation.atomicExchange"] = 225,
@@ -682,4 +682,6 @@ return {
682682
["Attr.MemoryScope"] = 678,
683683
["Undefined.LoadFromUninitializedMemory"] = 679,
684684
["CUDA_LDG"] = 680,
685+
["StoreBase.copyLogical"] = 681,
686+
["MakeStorageTypeLoweringConfig"] = 682,
685687
}

source/slang/slang-ir-insts.h

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,6 +1984,7 @@ struct IRUnconditionalBranch : IRTerminatorInst
19841984
IRUse block;
19851985

19861986
IRBlock* getTargetBlock() { return (IRBlock*)block.get(); }
1987+
IRUse* getTargetBlockUse() { return &block; }
19871988

19881989
UInt getArgCount();
19891990
IRUse* getArgs();
@@ -2590,7 +2591,10 @@ struct IRCastStorageToLogicalBase : IRInst
25902591
{
25912592
FIDDLE(baseInst())
25922593
IRInst* getVal() { return getOperand(0); }
2593-
IRInst* getBufferType() { return getOperand(1); }
2594+
IRMakeStorageTypeLoweringConfig* getLayoutConfig()
2595+
{
2596+
return as<IRMakeStorageTypeLoweringConfig>(getOperand(1));
2597+
}
25942598
};
25952599

25962600
FIDDLE()
@@ -3770,6 +3774,8 @@ struct IRBuilder
37703774
IRInst* emitStore(IRInst* dstPtr, IRInst* srcVal, IRInst* align);
37713775
IRInst* emitStore(IRInst* dstPtr, IRInst* srcVal, IRInst* align, IRInst* memoryScope);
37723776

3777+
IRInst* emitCopyLogical(IRInst* dest, IRInst* srcPtr, IRInst* instsToCopyLoadAttributesFrom);
3778+
37733779
IRInst* emitAtomicStore(IRInst* dstPtr, IRInst* srcVal, IRInst* memoryOrder);
37743780

37753781
IRInst* emitImageLoad(IRType* type, ShortList<IRInst*> params);
@@ -3977,8 +3983,18 @@ struct IRBuilder
39773983
IRInst* emitCastPtrToInt(IRInst* val);
39783984
IRInst* emitCastIntToPtr(IRType* ptrType, IRInst* val);
39793985

3980-
IRInst* emitCastStorageToLogical(IRType* type, IRInst* val, IRInst* bufferType);
3981-
IRInst* emitCastStorageToLogicalDeref(IRType* type, IRInst* val, IRInst* bufferType);
3986+
IRMakeStorageTypeLoweringConfig* emitMakeStorageTypeLoweringConfig(
3987+
AddressSpace addrspace,
3988+
IRTypeLayoutRuleName ruleName,
3989+
bool lowerToPhysicalType);
3990+
IRInst* emitCastStorageToLogical(
3991+
IRType* type,
3992+
IRInst* val,
3993+
IRMakeStorageTypeLoweringConfig* config);
3994+
IRInst* emitCastStorageToLogicalDeref(
3995+
IRType* type,
3996+
IRInst* val,
3997+
IRMakeStorageTypeLoweringConfig* config);
39823998

39833999
IRGlobalConstant* emitGlobalConstant(IRType* type);
39844000

source/slang/slang-ir-insts.lua

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -938,8 +938,16 @@ local insts = {
938938
{ field = { struct_name = "StructField", min_operands = 2 } },
939939
{ var = {} },
940940
{ load = { min_operands = 1 } },
941-
{ store = { min_operands = 2 } },
941+
{
942+
StoreBase =
943+
{
944+
operands = {{"ptr"}, {"val"}},
945+
{ store = {} },
946+
{ copyLogical = {} },
947+
},
948+
},
942949
{ CUDA_LDG = {min_operands = 1 } },
950+
943951
-- Atomic Operations
944952
{
945953
AtomicOperation = {
@@ -2335,8 +2343,10 @@ local insts = {
23352343
{ EnumCast = { operands = { { "value" } } } },
23362344
{ CastUInt2ToDescriptorHandle = { operands = { { "value" } } } },
23372345
{ CastDescriptorHandleToUInt2 = { operands = { { "value" } } } },
2338-
-- Represents a psuedo cast to convert between a logical type (user declared) and a storage Type
2346+
-- Represents a psuedo cast to convert between an original(user declared) type and a storage Type
23392347
-- (valid in buffer locations). The operand can either be a value or an address.
2348+
-- The first operand is a pointer to a storage type, the second operand must be a `MakeStorageTypeLoweringConfig` inst
2349+
-- that defines how the storage type is lowered from the original type.
23402350
{
23412351
CastStorageToLogicalBase = {
23422352
min_operands = 2,
@@ -2345,6 +2355,9 @@ local insts = {
23452355
{ CastStorageToLogicalDeref = { min_operands = 2, struct_name = "CastStorageToLogicalDeref" } },
23462356
},
23472357
},
2358+
-- IR encoding of a `TypeLoweringConfig` object that defines how a type is lowered to a storage type.
2359+
-- This is produced/consumed only in the lower-buffer-element-to-storage-type pass.
2360+
{ MakeStorageTypeLoweringConfig = { hoistable = true, operands = { { "addressSpace" }, { "layoutRule" }, { "lowerToPhysicalType" } } } },
23482361
{ CastUInt64ToDescriptorHandle = { operands = { { "value" } } } },
23492362
{ CastDescriptorHandleToUInt64 = { operands = { { "value" } } } },
23502363
-- Represents a no-op cast to convert a resource pointer to a resource on targets where the resource handles are

0 commit comments

Comments
 (0)