Bug 91124

Created attachment 116817 [details]
shader that triggers bug

The attached shader appears to trigger an issue in the FlatteningPass. Specifically the resulting code looks like

00000198: a0036003 00000000     joinat 0x1b0
000001a0: b00005fd 600187c8     set $c0 # ge f32 $r2 $r0
000001a8: 1000020d 0403c100     (e $c0) mov b32 $r3 $r1
000001b0: 10000801 2400c780   B ld $r0 b32 c0[0x10]

That joinat shouldn't be there at all. This is a bit tricky, since pre-RA, the code looks like

 60: joinat BB:5 (0)
 61: set u8 %c259 ge f32 %r215 %r255 (0)
 62: eq %c259 bra BB:4 (0)
BB:3 (1 instructions) - idom = BB:12, df = { BB:5 }
 -> BB:5 (forward)
 63: bra BB:5 (0)
BB:4 (1 instructions) - idom = BB:12, df = { BB:5 }
 -> BB:5 (forward)
 64: bra BB:5 (0)
BB:5 (14 instructions) - idom = BB:12, df = { BB:10 }
 -> BB:7 (tree)
 -> BB:6 (tree)
 65: phi u32 %r261 %r254 %r252 (0)
 66: join (0)

and the (e $c0) mov ... is actually generated from the phi node (since it creates constraint moves into BB:3/4. Post-RA but pre-FlatteningPass, this looks like

 64: joinat BB:5 (0)
 65: set u8 $c0 ge f32 $r2 $r0 (0)
 66: eq $c0 bra BB:4 (0)
BB:3 (1 instructions) - idom = BB:12, df = { BB:5 }
 -> BB:5 (forward)
 67: bra BB:5 (0)
BB:4 (2 instructions) - idom = BB:12, df = { BB:5 }
 -> BB:5 (forward)
 68: mov u32 $r3 $r1 (0)
 69: bra BB:5 (0)
BB:5 (13 instructions) - idom = BB:12, df = { BB:10 }
 -> BB:7 (tree)
 -> BB:6 (tree)
 70: join (0)

Will have to understand the FlatteningPass better before I can figure out what's wrong though.

I believe the following patch resolves the issue:

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp
index 51b9225..fa8ee07 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp
@@ -332,6 +332,9 @@ BasicBlock::splitBefore(Instruction *insn, bool attach)
    BasicBlock *bb = new BasicBlock(func);
    assert(!insn || insn->op != OP_PHI);
 
+   bb->joinAt = joinAt;
+   joinAt = NULL;
+
    splitCommon(insn, bb, attach);
    return bb;
 }

(In reply to Ilia Mirkin from comment #2)
> I believe the following patch resolves the issue:
> 
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp
> index 51b9225..fa8ee07 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp
> @@ -332,6 +332,9 @@ BasicBlock::splitBefore(Instruction *insn, bool attach)
>     BasicBlock *bb = new BasicBlock(func);
>     assert(!insn || insn->op != OP_PHI);
>  
> +   bb->joinAt = joinAt;
> +   joinAt = NULL;
> +
>     splitCommon(insn, bb, attach);
>     return bb;
>  }

The patch works very well, it fixes the reported issues in the game, thank you.

Pushed out as

commit 5dcb28c3d26828ed1b0e2bd5a0589c5baab04b85
Author: Ilia Mirkin <imirkin@alum.mit.edu>
Date:   Wed Jul 1 02:11:39 2015 -0400

    nv50/ir: copy joinAt when splitting both before and after