On 30/09/17 04:16, Ian Romanick wrote:
> From: Ian Romanick <ian.d.roman...@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.roman...@intel.com>
> ---
Thanks for the detailed explanation.
Reviewed-by: Alejandro Piñeiro <apinhe...@igalia.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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
