[Mesa-dev] [PATCH] glsl/ast: Stop processing a switch-statement after an error in the init-expression

[Ian Romanick]

From: Ian Romanick <ian.d.romanick at intel.com>

This happens to work now because ir_binop_all_equal is used.  This
causes vector typed init-expressions to produce scalar Boolean values
after comparison.

The next commit changes ir_binop_all_equal to ir_binop_equal.  Vector
typed init-expressions will then produce vector Boolean values, and, in
debug builds, the ir_assignment constructor will fail an assertion.

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
---
There were a number of flaws in the original series that went undetected
until recently.  One flaw, noted below, is fatal.  I had been testing
the series using our CI and shader-db.  The problems went undetected for
several reasons.  On the shader-db runs, I was using release builds that
omitted assertions.  At least one of the failures would have tri

The first failure was in changing ir_binop_all_equal to ir_binop_equal.
The code previously relied on ir_binop_all_equal accepting mixed scalar
and vector operands so that cases like

    switch (ivec2(a, b)) {
    case 0:
        break;
    }

would continue to compile.  An error would be emitted in the switch
init-statement, but the compiler would carry on trying to find more
errors.  That problem is addressed in this patch.  Once the error in the
switch init-statement is detected, halt compilation.

The second flaw was that if-to-conditional-assign did not respect the
write-mask of the assignment.  This would result in things like

    if (cond)
        foo.wz = bar;

being transformed to 'foo.wz = csel(cond.xx, bar, foo)'.  This was solved by
converting the write mask to a swizzle on the LHS when used in the
conditional-select.

The third flaw was that several of the patches that replace conditions
with conditional-select "forgot" that conditional-select is
per-component, so the vector size of the condition has to match the
vector size of the operands.

This last flaw led to the discovery of the fatal problem: ir_triop_csel
only works on scalar and vector operands.  Statements like:

    struct S s1;
    struct S s2;

    ...

    if (cond)
        s1 = s2;

cannot be translated directly to a conditional-select.  Arrays and
matrices have the same problem.  There are some possible solutions, but
I'm not going to have time to tackle any of them right now.

Assuming this patch meets with positive review, I intend to push a
reduced series of the subset listed below.  Aside from the ordering
change and the removal of 8 patches, the only change in the series is
the addition of this patch.  None of the older patches are modified.
This is still a nice clean up and a net removal of 56 lines of code.
It's not the payoff I wanted, but I guess it's better than nothing.

glsl: Fix coding standards issues in lower_if_to_cond_assign
glsl: Fix coding standards issues in lower_vec_index_to_cond_assign
glsl: Return ir_variable from compare_index_block
glsl: Convert lower_vec_index_to_cond_assign to using ir_builder
glsl: Fix coding standards issues in lower_variable_index_to_cond_assign
glsl: Convert lower_variable_index_to_cond_assign to ir_builder
glsl: Don't pass NULL to ir_assignment constructor when not necessary
glsl/ast: Stop processing a switch-statement after an error in the init-expression
glsl/ast: Use ir_binop_equal instead of ir_binop_all_equal
glsl/ast: Convert ast_case_label::hir to ir_builder
glsl/ast: Explicitly track the set of case labels that occur after default
glsl/ast: Generate a more compact expression to disable execution of default case
glsl/ast: Use logical-or instead of conditional assignment to set fallthru_var
glsl: Move 'foo = foo;' optimization to opt_dead_code_local
glsl: Remove spurious assertions

This, along with the next series I intend to send out, is available at:

https://cgit.freedesktop.org/~idr/mesa/log/?h=glsl-parser-diet

 src/compiler/glsl/ast_to_hir.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index c464549..a569318 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6418,6 +6418,7 @@ ast_switch_statement::hir(exec_list *instructions,
                        state,
                        "switch-statement expression must be scalar "
                        "integer");
+      return NULL;
    }
 
    /* Track the switch-statement nesting in a stack-like manner.
-- 
2.9.5