From 112caca00ba9bfd9e1051bb94969efa9e74c6c03 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Thu, 8 Feb 2018 07:54:04 -0800 Subject: Falcor fixes (#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 --- source/slang/ir.cpp | 69 +++++++++++++++-------------------------------------- 1 file changed, 19 insertions(+), 50 deletions(-) (limited to 'source/slang/ir.cpp') diff --git a/source/slang/ir.cpp b/source/slang/ir.cpp index 634632f0d..824af64ae 100644 --- a/source/slang/ir.cpp +++ b/source/slang/ir.cpp @@ -170,8 +170,6 @@ namespace Slang break; case kIROp_unconditionalBranch: - case kIROp_break: - case kIROp_continue: case kIROp_loop: // unconditonalBranch begin = args + 0; @@ -179,9 +177,7 @@ namespace Slang break; case kIROp_conditionalBranch: - case kIROp_if: case kIROp_ifElse: - case kIROp_loopTest: // conditionalBranch begin = args + 1; end = begin + 2; @@ -401,12 +397,8 @@ namespace Slang case kIROp_ReturnVoid: case kIROp_unconditionalBranch: case kIROp_conditionalBranch: - case kIROp_break: - case kIROp_continue: case kIROp_loop: - case kIROp_if: case kIROp_ifElse: - case kIROp_loopTest: case kIROp_discard: case kIROp_switch: case kIROp_unreachable: @@ -1302,6 +1294,15 @@ namespace Slang return nullptr; } + // Ugly special case: the result of loading from `groupshared` + // memory should not itself be `groupshared`. + // + // TODO: Should this generalize to any "rate-qualified" type? + if(auto rateType = valueType->As()) + { + valueType = rateType->valueType; + } + auto inst = createInst( this, kIROp_Load, @@ -1526,25 +1527,13 @@ namespace Slang IRInst* IRBuilder::emitBreak( IRBlock* target) { - auto inst = createInst( - this, - kIROp_break, - nullptr, - target); - addInst(inst); - return inst; + return emitBranch(target); } IRInst* IRBuilder::emitContinue( IRBlock* target) { - auto inst = createInst( - this, - kIROp_continue, - nullptr, - target); - addInst(inst); - return inst; + return emitBranch(target); } IRInst* IRBuilder::emitLoop( @@ -1583,17 +1572,18 @@ namespace Slang return inst; } - IRInst* IRBuilder::emitIf( + IRInst* IRBuilder::emitIfElse( IRValue* val, IRBlock* trueBlock, + IRBlock* falseBlock, IRBlock* afterBlock) { - IRValue* args[] = { val, trueBlock, afterBlock }; + IRValue* args[] = { val, trueBlock, falseBlock, afterBlock }; UInt argCount = sizeof(args) / sizeof(args[0]); - auto inst = createInst( + auto inst = createInst( this, - kIROp_if, + kIROp_ifElse, nullptr, argCount, args); @@ -1601,23 +1591,12 @@ namespace Slang return inst; } - IRInst* IRBuilder::emitIfElse( + IRInst* IRBuilder::emitIf( IRValue* val, IRBlock* trueBlock, - IRBlock* falseBlock, IRBlock* afterBlock) { - IRValue* args[] = { val, trueBlock, falseBlock, afterBlock }; - UInt argCount = sizeof(args) / sizeof(args[0]); - - auto inst = createInst( - this, - kIROp_ifElse, - nullptr, - argCount, - args); - addInst(inst); - return inst; + return emitIfElse(val, trueBlock, afterBlock, afterBlock); } IRInst* IRBuilder::emitLoopTest( @@ -1625,17 +1604,7 @@ namespace Slang IRBlock* bodyBlock, IRBlock* breakBlock) { - IRValue* args[] = { val, bodyBlock, breakBlock }; - UInt argCount = sizeof(args) / sizeof(args[0]); - - auto inst = createInst( - this, - kIROp_loopTest, - nullptr, - argCount, - args); - addInst(inst); - return inst; + return emitIfElse(val, bodyBlock, breakBlock, bodyBlock); } IRInst* IRBuilder::emitSwitch( -- cgit v1.2.3