Skip to content

Commit e1d0ef2

Browse files
ArielG-NVArielG-NV
authoredJun 26, 2024
Expand upon existing ImageSubscript support (Metal, GLSL, SPIRV) (shader-slang#4408)
* Add additional `ImageSubscript` features: 1. Added ImageSubscript support for Metal & a test case * Merge GLSL/SPIRV/Metal `ImageSubscript` legalization pass 2. Added multisample support to glsl/spirv/metal for when using ImageSubscript * Added in this PR since the overhaul of the code merges together GLSL/SPIRV/Metal implementation 3. Fixed minor metal texture `Load`/`Read` bugs * [HLSL methods of access do not support subscript accessor for texture cube array](https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/texturecubearray) * removed swizzling of uint/int/float * other odd bugs which were causing compile errors note: Compute tests do not work due to what seems to be the GFX backend (causes crash without error report). The tests are disabled. * disable LOD with texture 1d seems that LOD for 1d textures need to be a compile time constant as per an error metal throws * syntax error in hlsl.meta * static_assert alone with intrinsic_asm error provides cleaner errors Note: `static_assert` seems to be unstable and not be fully respected (still require `intrinsic_asm` to avoid a stdlib compile error) * change comment to `// lod is not supported for 1D texture * add `static_assert` in related code gen paths * address review * address review * add asserts as per review comment, NOTE: unclear if these should be release 'asserts' as well
1 parent 969dd4c commit e1d0ef2

18 files changed

+472
-181
lines changed
 

‎build/visual-studio/slang/slang.vcxproj

+2
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ IF EXIST ..\..\..\external\slang-glslang\bin\windows-aarch64\release\slang-glsla
422422
<ClInclude Include="..\..\..\source\slang\slang-ir-insts.h" />
423423
<ClInclude Include="..\..\..\source\slang\slang-ir-layout.h" />
424424
<ClInclude Include="..\..\..\source\slang\slang-ir-legalize-array-return-type.h" />
425+
<ClInclude Include="..\..\..\source\slang\slang-ir-legalize-image-subscript.h" />
425426
<ClInclude Include="..\..\..\source\slang\slang-ir-legalize-mesh-outputs.h" />
426427
<ClInclude Include="..\..\..\source\slang\slang-ir-legalize-uniform-buffer-load.h" />
427428
<ClInclude Include="..\..\..\source\slang\slang-ir-legalize-varying-params.h" />
@@ -665,6 +666,7 @@ IF EXIST ..\..\..\external\slang-glslang\bin\windows-aarch64\release\slang-glsla
665666
<ClCompile Include="..\..\..\source\slang\slang-ir-insert-debug-value-store.cpp" />
666667
<ClCompile Include="..\..\..\source\slang\slang-ir-layout.cpp" />
667668
<ClCompile Include="..\..\..\source\slang\slang-ir-legalize-array-return-type.cpp" />
669+
<ClCompile Include="..\..\..\source\slang\slang-ir-legalize-image-subscript.cpp" />
668670
<ClCompile Include="..\..\..\source\slang\slang-ir-legalize-mesh-outputs.cpp" />
669671
<ClCompile Include="..\..\..\source\slang\slang-ir-legalize-types.cpp" />
670672
<ClCompile Include="..\..\..\source\slang\slang-ir-legalize-uniform-buffer-load.cpp" />

‎build/visual-studio/slang/slang.vcxproj.filters

+6
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,9 @@
354354
<ClInclude Include="..\..\..\source\slang\slang-ir-legalize-array-return-type.h">
355355
<Filter>Header Files</Filter>
356356
</ClInclude>
357+
<ClInclude Include="..\..\..\source\slang\slang-ir-legalize-image-subscript.h">
358+
<Filter>Header Files</Filter>
359+
</ClInclude>
357360
<ClInclude Include="..\..\..\source\slang\slang-ir-legalize-mesh-outputs.h">
358361
<Filter>Header Files</Filter>
359362
</ClInclude>
@@ -1079,6 +1082,9 @@
10791082
<ClCompile Include="..\..\..\source\slang\slang-ir-legalize-array-return-type.cpp">
10801083
<Filter>Source Files</Filter>
10811084
</ClCompile>
1085+
<ClCompile Include="..\..\..\source\slang\slang-ir-legalize-image-subscript.cpp">
1086+
<Filter>Source Files</Filter>
1087+
</ClCompile>
10821088
<ClCompile Include="..\..\..\source\slang\slang-ir-legalize-mesh-outputs.cpp">
10831089
<Filter>Source Files</Filter>
10841090
</ClCompile>

‎source/slang/hlsl.meta.slang

+13-18
Original file line numberDiff line numberDiff line change
@@ -2416,11 +2416,12 @@ extension __TextureImpl<T,Shape,isArray,0,sampleCount,0,isShadow,isCombined,form
24162416
__intrinsic_asm "$c$0.read(vec<uint,3>(($1).xyz), uint(($1).w))$z";
24172417
break;
24182418
case $(SLANG_TEXTURE_CUBE):
2419+
static_assert(isArray == 0, "Unsupported 'Load' of 'texture cube array' for 'metal' target");
24192420
if (isShadow == 1)
24202421
{
24212422
if (isArray == 1)
24222423
// T read(uint2 coord, uint face, uint array, uint lod = 0) const
2423-
__intrinsic_asm "$0.read(vec<uint,2>(($1).xy), uint(($1).z), uint(($1).w))";
2424+
__intrinsic_asm "<invalid intrinsics>";
24242425
else
24252426
// T read(uint2 coord, uint face, uint lod = 0) const
24262427
__intrinsic_asm "$c$0.read(vec<uint,2>(($1).xy), uint(($1).z), uint(($1).w))$z";
@@ -2429,14 +2430,14 @@ extension __TextureImpl<T,Shape,isArray,0,sampleCount,0,isShadow,isCombined,form
24292430
{
24302431
if (isArray == 1)
24312432
// Tv read(uint2 coord, uint face, uint array, uint lod = 0) const
2432-
__intrinsic_asm "$0.read(vec<uint,2>(($1).xy), uint(($1).z), uint(($1).w))";
2433+
__intrinsic_asm "<invalid intrinsics>";
24332434
else
24342435
// Tv read(uint2 coord, uint face, uint lod = 0) const
24352436
__intrinsic_asm "$c$0.read(vec<uint,2>(($1).xy), uint(($1).z), uint(($1).w))$z";
24362437
}
24372438
break;
24382439
}
2439-
// TODO: This needs to be handled by the capability system
2440+
static_assert(false, "Unsupported 'Load' of 'texture' for 'metal' target");
24402441
__intrinsic_asm "<invalid intrinsics>";
24412442
case glsl:
24422443
__intrinsic_asm "$ctexelFetch($0, ($1).$w1b, ($1).$w1e)$z";
@@ -2819,6 +2820,7 @@ extension __TextureImpl<T,Shape,isArray,0,sampleCount,$(access),isShadow, 0,form
28192820
__intrinsic_asm "$c$0.read(vec<uint,3>(($1).xyz))$z";
28202821
break;
28212822
case $(SLANG_TEXTURE_CUBE):
2823+
static_assert(isArray == 0, "Unsupported 'Load' of 'texture cube array' for 'metal' target");
28222824
if (isShadow == 1)
28232825
{
28242826
if (isArray == 1)
@@ -2839,7 +2841,7 @@ extension __TextureImpl<T,Shape,isArray,0,sampleCount,$(access),isShadow, 0,form
28392841
}
28402842
break;
28412843
}
2842-
// TODO: This needs to be handled by the capability system
2844+
static_assert(false, "Unsupported 'Load' of 'texture' for 'metal' target");
28432845
__intrinsic_asm "<invalid intrinsics>";
28442846
}
28452847
}
@@ -2960,10 +2962,10 @@ extension __TextureImpl<T,Shape,isArray,0,sampleCount,$(access),isShadow, 0,form
29602962
// lod is not supported for 1D texture
29612963
if (isArray == 1)
29622964
// void write(Tv color, uint coord, uint array, uint lod = 0) const
2963-
__intrinsic_asm "$0.write($2, uint(($1).x))";
2965+
__intrinsic_asm "$0.write($2, uint(($1).x), uint(($1).y))";
29642966
else
29652967
// void write(Tv color, uint coord, uint lod = 0) const
2966-
__intrinsic_asm "$0.write($2, uint(($1).x))";
2968+
__intrinsic_asm "$0.write($2, uint($1))";
29672969
break;
29682970
case $(SLANG_TEXTURE_2D):
29692971
if (isShadow == 1)
@@ -2991,23 +2993,16 @@ extension __TextureImpl<T,Shape,isArray,0,sampleCount,$(access),isShadow, 0,form
29912993
__intrinsic_asm "$0.write($2, vec<uint,3>(($1).xyz))";
29922994
break;
29932995
case $(SLANG_TEXTURE_CUBE):
2996+
static_assert(isArray == 0, "Unsupported 'Store' of 'texture cube array' for 'metal' target");
29942997
if (isShadow == 1)
29952998
{
2996-
if (isArray == 1)
2997-
// void write(Tv color, uint2 coord, uint face, uint array, uint lod = 0) const
2998-
__intrinsic_asm "$0.write($2, vec<uint,2>(($1).xy), uint(($1).z)%6, uint(($1).z)/6)";
2999-
else
3000-
// void write(Tv color, uint2 coord, uint face, uint lod = 0) const
3001-
__intrinsic_asm "$0.write($2, vec<uint,2>(($1).xy), uint(($1).z))";
2999+
// void write(Tv color, uint2 coord, uint face, uint lod = 0) const
3000+
__intrinsic_asm "$0.write($2, vec<uint,2>(($1).xy), uint(($1).z), uint(($1).w))";
30023001
}
30033002
else
30043003
{
3005-
if (isArray == 1)
3006-
// void write(Tv color, uint2 coord, uint face, uint array, uint lod = 0) const
3007-
__intrinsic_asm "$0.write($2, vec<uint,2>(($1).xy), uint(($1).z)%6, uint(($1).z)/6)";
3008-
else
3009-
// void write(Tv color, uint2 coord, uint face, uint lod = 0) const
3010-
__intrinsic_asm "$0.write($2, vec<uint,2>(($1).xy), uint(($1).z))";
3004+
// void write(Tv color, uint2 coord, uint face, uint lod = 0) const
3005+
__intrinsic_asm "$0.write($2, vec<uint,2>(($1).xy), uint(($1).z), uint(($1).w))";
30113006
}
30123007
break;
30133008
}

‎source/slang/slang-diagnostic-defs.h

+2
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,8 @@ DIAGNOSTIC(41400, Error, staticAssertionFailure, "static assertion failed, $0")
774774
DIAGNOSTIC(41401, Error, staticAssertionFailureWithoutMessage, "static assertion failed.")
775775
DIAGNOSTIC(41402, Error, staticAssertionConditionNotConstant, "condition for static assertion cannot be evaluated at the compile-time.")
776776

777+
DIAGNOSTIC(41402, Error, multiSampledTextureDoesNotAllowWrites, "cannot write to a multisampled texture with target '$0'.")
778+
777779
//
778780
// 5xxxx - Target code generation.
779781
//

‎source/slang/slang-emit-glsl.cpp

+17-5
Original file line numberDiff line numberDiff line change
@@ -2021,21 +2021,33 @@ bool GLSLSourceEmitter::tryEmitInstExprImpl(IRInst* inst, const EmitOpInfo& inOu
20212021
}
20222022
case kIROp_ImageLoad:
20232023
{
2024+
auto imageOp = as<IRImageLoad>(inst);
20242025
m_writer->emit("imageLoad(");
2025-
emitOperand(inst->getOperand(0), getInfo(EmitOp::General));
2026+
emitOperand(imageOp->getImage(), getInfo(EmitOp::General));
20262027
m_writer->emit(",");
2027-
emitOperand(inst->getOperand(1), getInfo(EmitOp::General));
2028+
emitOperand(imageOp->getCoord(), getInfo(EmitOp::General));
2029+
if (imageOp->hasAuxCoord1())
2030+
{
2031+
m_writer->emit(",");
2032+
emitOperand(imageOp->getAuxCoord1(), getInfo(EmitOp::General));
2033+
}
20282034
m_writer->emit(")");
20292035
return true;
20302036
}
20312037
case kIROp_ImageStore:
20322038
{
2039+
auto imageOp = as<IRImageStore>(inst);
20332040
m_writer->emit("imageStore(");
2034-
emitOperand(inst->getOperand(0), getInfo(EmitOp::General));
2041+
emitOperand(imageOp->getImage(), getInfo(EmitOp::General));
20352042
m_writer->emit(",");
2036-
emitOperand(inst->getOperand(1), getInfo(EmitOp::General));
2043+
emitOperand(imageOp->getCoord(), getInfo(EmitOp::General));
2044+
if (imageOp->hasAuxCoord1())
2045+
{
2046+
m_writer->emit(",");
2047+
emitOperand(imageOp->getAuxCoord1(), getInfo(EmitOp::General));
2048+
}
20372049
m_writer->emit(",");
2038-
emitOperand(inst->getOperand(2), getInfo(EmitOp::General));
2050+
emitOperand(imageOp->getValue(), getInfo(EmitOp::General));
20392051
m_writer->emit(")");
20402052
return true;
20412053
}

‎source/slang/slang-emit-metal.cpp

+41
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,47 @@ bool MetalSourceEmitter::tryEmitInstExprImpl(IRInst* inst, const EmitOpInfo& inO
451451
emitOperand(inst->getOperand(2), getInfo(EmitOp::General));
452452
return true;
453453
}
454+
case kIROp_ImageLoad:
455+
{
456+
auto imageOp = as<IRImageLoad>(inst);
457+
emitOperand(imageOp->getImage(), getInfo(EmitOp::General));
458+
m_writer->emit(".read(");
459+
emitOperand(imageOp->getCoord(), getInfo(EmitOp::General));
460+
if(imageOp->hasAuxCoord1())
461+
{
462+
m_writer->emit(",");
463+
emitOperand(imageOp->getAuxCoord1(), getInfo(EmitOp::General));
464+
}
465+
if(imageOp->hasAuxCoord2())
466+
{
467+
m_writer->emit(",");
468+
emitOperand(imageOp->getAuxCoord2(), getInfo(EmitOp::General));
469+
}
470+
m_writer->emit(")");
471+
return true;
472+
}
473+
case kIROp_ImageStore:
474+
{
475+
476+
auto imageOp = as<IRImageStore>(inst);
477+
emitOperand(imageOp->getImage(), getInfo(EmitOp::General));
478+
m_writer->emit(".write(");
479+
emitOperand(imageOp->getValue(), getInfo(EmitOp::General));
480+
m_writer->emit(",");
481+
emitOperand(imageOp->getCoord(), getInfo(EmitOp::General));
482+
if(imageOp->hasAuxCoord1())
483+
{
484+
m_writer->emit(",");
485+
emitOperand(imageOp->getAuxCoord1(), getInfo(EmitOp::General));
486+
}
487+
if(imageOp->hasAuxCoord2())
488+
{
489+
m_writer->emit(",");
490+
emitOperand(imageOp->getAuxCoord2(), getInfo(EmitOp::General));
491+
}
492+
m_writer->emit(")");
493+
return true;
494+
}
454495
default: break;
455496
}
456497
// Not handled

‎source/slang/slang-emit-spirv.cpp

+16-2
Original file line numberDiff line numberDiff line change
@@ -2989,12 +2989,26 @@ struct SPIRVEmitContext
29892989

29902990
SpvInst* emitImageLoad(SpvInstParent* parent, IRImageLoad* load)
29912991
{
2992-
return emitInst(parent, load, SpvOpImageRead, load->getDataType(), kResultID, load->getImage(), load->getCoord());
2992+
if (load->hasAuxCoord1())
2993+
{
2994+
return emitInst(parent, load, SpvOpImageRead, load->getDataType(), kResultID, load->getImage(), load->getCoord(), SpvImageOperandsSampleMask, load->getAuxCoord1());
2995+
}
2996+
else
2997+
{
2998+
return emitInst(parent, load, SpvOpImageRead, load->getDataType(), kResultID, load->getImage(), load->getCoord());
2999+
}
29933000
}
29943001

29953002
SpvInst* emitImageStore(SpvInstParent* parent, IRImageStore* store)
29963003
{
2997-
return emitInst(parent, store, SpvOpImageWrite, store->getImage(), store->getCoord(), store->getValue());
3004+
if (store->hasAuxCoord1())
3005+
{
3006+
return emitInst(parent, store, SpvOpImageWrite, store->getImage(), store->getCoord(), store->getValue(), SpvImageOperandsSampleMask, store->getAuxCoord1());
3007+
}
3008+
else
3009+
{
3010+
return emitInst(parent, store, SpvOpImageWrite, store->getImage(), store->getCoord(), store->getValue());
3011+
}
29983012
}
29993013

30003014
SpvInst* emitImageSubscript(SpvInstParent* parent, IRImageSubscript* subscript)

‎source/slang/slang-emit.cpp

+17-2
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
#include "slang-ir-lower-l-value-cast.h"
5151
#include "slang-ir-lower-reinterpret.h"
5252
#include "slang-ir-loop-unroll.h"
53+
#include "slang-ir-legalize-image-subscript.h"
5354
#include "slang-ir-legalize-vector-types.h"
5455
#include "slang-ir-metadata.h"
5556
#include "slang-ir-optix-entry-point-uniforms.h"
@@ -1153,14 +1154,28 @@ Result linkAndOptimizeIR(
11531154
if(isD3DTarget(targetRequest))
11541155
legalizeNonStructParameterToStructForHLSL(irModule);
11551156

1156-
// Legalize `ImageSubscript` and constant buffer loads for GLSL.
1157+
// Legalize `ImageSubscript` loads.
1158+
switch (target)
1159+
{
1160+
case CodeGenTarget::Metal:
1161+
case CodeGenTarget::GLSL:
1162+
case CodeGenTarget::SPIRV:
1163+
case CodeGenTarget::SPIRVAssembly:
1164+
{
1165+
legalizeImageSubscript(targetRequest, irModule, sink);
1166+
}
1167+
break;
1168+
default:
1169+
break;
1170+
}
1171+
1172+
// Legalize constant buffer loads.
11571173
switch (target)
11581174
{
11591175
case CodeGenTarget::GLSL:
11601176
case CodeGenTarget::SPIRV:
11611177
case CodeGenTarget::SPIRVAssembly:
11621178
{
1163-
legalizeImageSubscriptForGLSL(irModule);
11641179
legalizeConstantBufferLoadForGLSL(irModule);
11651180
legalizeDispatchMeshPayloadForGLSL(irModule);
11661181
}

0 commit comments

Comments
 (0)