Skip to content

Commit 112caca

Browse files
author
Tim Foley
authored
Falcor fixes (shader-slang#402)
* Re-define deprecated compile flags By including these flags in the header file, with a value of zero, we can allow some existing code to compile even after the major changes to the implementation. * The `SLANG_COMPILE_FLAG_NO_CHECKING` option will effectively be ignored, since checking is always enabled. * The `SLANG_COMPILE_FLAG_SPLIT_MIXED_TYPES` option will now act as if it is always enabled (and indeed some of the code has been relying on this flag being set always). * Make subscript operators writable for writable textures This even had a `TODO` comment saying that we needed to fix it, and now I'm seeing semantic checking failures because we didn't define these and so we find assignment to non l-values. * Fix definitions of any() and all() intrinsics These should always return a scalar `bool` value, but they were being defined wrong in two ways: 1. They were using their generic type parameter `T` in the return type 2. They were returning a vector in the vector case, and a matrix in the matrix case. This change just alters the return type to be `bool` in all cases. * Fix bug in SSA construction When eliminating a trivial phi node, it is possible that the phi is still recorded as the "latest" value for a local variable in its block. When later code queries that value from the block (which can happen whenever another block looks up a variable in its predecessors), it would get the old phi and not the replacement value. I simply added a loop that checks if the value we look up is a phi that got replaced, and then continues with the replacement value (which might itself be a phi...). A more advanced solution might try to get clever and have the map itself hold `IRUse` values so that we can replace them seamlessly. * Simplify IR control flow representation This change gets rid of various special-case operations for conditional and unconditional branches, and instead requires emit logic to recognize when a direct branch is targetting a `break` or `continue` label. The new approach here isn't perfect, but it seems beter than what we had before, because it can actually work in the presence of control-flow optimizations (including our current critical-edge-splitting step). * Load from groupshared isn't groupshared When loading from a `groupshared` variable, the resulting temporary shouldn't have the `groupshared` qualifier on it. This might eventually need to generalize to a better understanding of storage modifiers in the IR, but I don't really want to deal with that right now. * Don't emit references to typedefs in output code Now that we are using the IR for all codegen, we shouldn't be dealing with surface-level things like `typedef` declarations in the output code; just use the type that was being referred to in the first place. * Fix floating-point literal printing for IR The IR was calling `emit()` instead of `Emit()` (we really need to normalize our convention here), and was implicitly invoking a default constructor on `String` that takes a `double` (that constructor should really be marked `explicit`), and which doesn't meet our requirements for printing floating-point values. * Fix error when importing module that doesn't parse We already added a case to bail out if semantic checking fails, but neglected to add a case if there is an error during parsing of a module to be imported. Note: this logic doesn't correctly register the module as being loaded (but still in error), so users could see multiple error messages if there are multiple `import`s for the same module. * Improve error message for overload resolution failure - Drop debugging info from the candidate printing - Add cases to print `double` and `half` types properly * Fixup: switch loopTest to ifElse in expected IR output
1 parent be8b891 commit 112caca

14 files changed

+277
-198
lines changed

slang.h

+6
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ extern "C"
127127

128128
/* Skip code generation step, just check the code and generate layout */
129129
SLANG_COMPILE_FLAG_NO_CODEGEN = 1 << 4,
130+
131+
/* Deprecated flags: kept around to allow existing applications to
132+
compmile. Note that the relevant features will still be left in
133+
their default state. */
134+
SLANG_COMPILE_FLAG_NO_CHECKING = 0,
135+
SLANG_COMPILE_FLAG_SPLIT_MIXED_TYPES = 0,
130136
};
131137

132138
/*!

source/slang/check.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -5961,6 +5961,8 @@ namespace Slang
59615961

59625962
// declString = declString + "[" + String(candidate.conversionCostSum) + "]";
59635963

5964+
#if 0
5965+
// Debugging: ensure that we don't consider multiple declarations of the same operation
59645966
if (auto decl = dynamic_cast<CallableDecl*>(candidate.item.declRef.decl))
59655967
{
59665968
char buffer[1024];
@@ -5970,6 +5972,7 @@ namespace Slang
59705972
decl->nextDecl);
59715973
declString.append(buffer);
59725974
}
5975+
#endif
59735976

59745977
getSink()->diagnose(candidate.item.declRef, Diagnostics::overloadCandidate, declString);
59755978

source/slang/core.meta.slang

+16-4
Original file line numberDiff line numberDiff line change
@@ -627,16 +627,28 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
627627

628628
if(baseShape != TextureType::ShapeCube)
629629
{
630-
// TODO: In the case where `access` includes writeability,
631-
// this should have both `get` and `set` accessors.
632-
633630
// subscript operator
634631
sb << "__subscript(uint";
635632
if(kBaseTextureTypes[tt].coordCount + isArray > 1)
636633
{
637634
sb << kBaseTextureTypes[tt].coordCount + isArray;
638635
}
639-
sb << " location) -> T;\n";
636+
sb << " location) -> T";
637+
638+
// Depending on the access level of the texture type,
639+
// we either have just a getter (the default), or both
640+
// a getter and setter.
641+
switch( access )
642+
{
643+
case SLANG_RESOURCE_ACCESS_NONE:
644+
case SLANG_RESOURCE_ACCESS_READ:
645+
sb << ";\n";
646+
break;
647+
648+
default:
649+
sb << " { get; set; }\n";
650+
break;
651+
}
640652
}
641653

642654
if( !isMultisample )

source/slang/core.meta.slang.h

+16-4
Original file line numberDiff line numberDiff line change
@@ -630,16 +630,28 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt)
630630

631631
if(baseShape != TextureType::ShapeCube)
632632
{
633-
// TODO: In the case where `access` includes writeability,
634-
// this should have both `get` and `set` accessors.
635-
636633
// subscript operator
637634
sb << "__subscript(uint";
638635
if(kBaseTextureTypes[tt].coordCount + isArray > 1)
639636
{
640637
sb << kBaseTextureTypes[tt].coordCount + isArray;
641638
}
642-
sb << " location) -> T;\n";
639+
sb << " location) -> T";
640+
641+
// Depending on the access level of the texture type,
642+
// we either have just a getter (the default), or both
643+
// a getter and setter.
644+
switch( access )
645+
{
646+
case SLANG_RESOURCE_ACCESS_NONE:
647+
case SLANG_RESOURCE_ACCESS_READ:
648+
sb << ";\n";
649+
break;
650+
651+
default:
652+
sb << " { get; set; }\n";
653+
break;
654+
}
643655
}
644656

645657
if( !isMultisample )

0 commit comments

Comments
 (0)