On Wed, 2019-02-13 at 11:53 -0800, Ian Romanick wrote: > On 2/13/19 9:59 AM, Juan A. Suarez Romero wrote: > > On Wed, 2019-02-13 at 09:16 -0800, Ian Romanick wrote: > > > On 2/13/19 7:53 AM, Juan A. Suarez Romero wrote: > > > > On Tue, 2019-02-12 at 16:22 -0800, Ian Romanick wrote: > > > > > On 2/12/19 12:58 AM, Juan A. Suarez Romero wrote: > > > > > > opt_split_alu_of_phi moves ALU instruction to the end of > > > > > > continue block. > > > > > > > > > > > > But if the continue block ends with a jump instruction (an > > > > > > explicit > > > > > > "continue" instruction) then the ALU must be inserted > > > > > > before the jump, > > > > > > as it is illegal to add instructions after the jump. > > > > > > > > > > I'm assuming you found this by inspection? Since this pass > > > > > only > > > > > operates when the first block of the loop only has two > > > > > predecessors (the > > > > > block before the loop and the implicit continue at the end of > > > > > the loop), > > > > > this shouldn't be a a problem in practice... or were you able > > > > > to trigger > > > > > it somehow? > > > > > > > > Found when dealing with the SPIR-V code that I've sent in > > > > https://lists.freedesktop.org/archives/mesa-dev/2019-February/214906.html > > > > > > > > > > > > The obtained NIR code has an explicit continue at the end of > > > > the loop (see > > > > http://paste.debian.net/1067619/, in particular the loop with > > > > header block_2). > > > > > > That loop is a mess. Wow... I'm impressed. :) I see a continue > > > inside > > > an else-clause at line 107 and a break at line 112. I now > > > understand > > > the fundamental problem. Am I correct that this problem would > > > also > > > occur before 8fb8ebfbb05? And that's what "nir: allow stitching > > > of > > > non-empty block" fixes? > > > > Yeah :) It's a SPIR-V code generated with some fuzzy tool. > > > > > > And yes, even with this patch applied, it will break later when > > stitching the > > code. > > > > Fortunately, I got rid of the "nir: allow stitching of non-empty > > block" patch > > and sent instead another one that removes one of the jumps to avoid > > the issue. > > > > > Did nir_validate trip on this? If not, it seems like it > > > should... > > > though that might cause problems when nir_validate is run > > > immediately > > > following translation into NIR. > > > > The NIR code seems valid. The error (assert) was caught by > > nir_instr_insert, > > when applying the optimization. > > Having instructions in a block after an unconditional jump seems > bogus. > It's technically valid, but it can never be correct. I'm pretty > sure > we scrub that from the GLSL frontend, and I'm not sure it's legal in > SPIR-V. I'll look into it more. >
It's definitely illegal in SPIR-V, as *all* control flow instructions terminate blocks. You would have to start a new block (by inserting a label) to make it legal. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
