Skip to content
Permalink
Browse files

Fix `exit` not being recognized as basic block end (#1051)

Stale assignment elision was broken in the presence of `exit`.

In general

    a = 10;
    ... stuff not involving `a`...
    a = 20;

sees two assignments within the same basic block, and because `a` is not used between the two assignments, the first one can be eliminated.

But this optimization ("stale assignment elision") was wrong for this modified code:

    a = 10;
    return;  // note: `return` in the main shader body is the same as `exit()`
    a = 20;

because the `return` changes things -- the second assignment won't be executed, and therefore should not retroactively eliminate the earlier assignment. The correct final value of a is indeed 10.

The problem boiled down to a bug in OSOProcessorBase::find_basic_blocks, where the `exit` op was simply not recognized as terminating its basic block (since it is a jump).
  • Loading branch information...
marsupial authored and lgritz committed Aug 15, 2019
1 parent 3635cd0 commit 5ee8f866e25c11efc4ad5fb1a876471915867e31
@@ -304,8 +304,8 @@ if (OSL_BUILD_TESTS)
TESTSUITE ( aastep allowconnect-err and-or-not-synonyms arithmetic
array array-derivs array-range array-aassign
blackbody blendmath breakcont
bug-array-heapoffsets
bug-locallifetime bug-outputinit bug-param-duplicate bug-peep
bug-array-heapoffsets bug-locallifetime bug-outputinit
bug-param-duplicate bug-peep bug-return
cellnoise closure closure-array color comparison
compile-buffer
component-range
@@ -43,6 +43,7 @@ using namespace OSL::pvt;

// names of ops we'll be using frequently
static ustring u_nop ("nop"),
u_exit ("exit"),
u_assign ("assign"),
u_add ("add"),
u_sub ("sub"),
@@ -1081,10 +1082,10 @@ OSOProcessorBase::find_basic_blocks ()
// any jump targets at all, it must be a conditional or loop.
if (op.jump(0) >= 0)
block_begin[opnum+1] = true;
// 'break', 'continue', and 'return' also cause the next
// 'break', 'continue', 'return', and 'exit' also cause the next
// statement to begin a new basic block.
if (op.opname() == u_break || op.opname() == u_continue ||
op.opname() == u_return)
op.opname() == u_return || op.opname() == u_exit)
block_begin[opnum+1] = true;
}

Binary file not shown.
@@ -0,0 +1,3 @@
Compiled test.osl -> test.oso

Output Cout to out.tif
@@ -0,0 +1,4 @@
#!/usr/bin/env python

command += testshade("-g 1 1 --center -od uint8 -o Cout out.tif test")
outputs = [ "out.txt", "out.tif" ]
@@ -0,0 +1,11 @@
// Regression for a early return bug which nops the first assignment in favor of an unreachable one

shader
test (output color Cout = 0)
{
Cout = color (0, 1, 0);
return;

Cout = color(1, 0, 0);
return;
}

0 comments on commit 5ee8f86

Please sign in to comment.
You can’t perform that action at this time.