For the series:
Reviewed-by: Marek Olšák <marek.ol...@amd.com>
Marek
On Fri, Dec 14, 2018 at 12:33 AM Timothy Arceri <tarc...@itsqueeze.com>
wrote:
> The previous code used a do while loop and continues after walking
> a nested loop/if-statement. This means we end up evaluating the
> last instruction from the nested block against the while condition
> and potentially exit early if it matches the exit condition of the
> outer block.
>
> Fixes: 386d165d8d09 ("tgsi/scan: add a new pass that analyzes tess factor
> writes")
> ---
> No changes in shader-db. Didn't run against piglit.
>
> src/gallium/auxiliary/tgsi/tgsi_scan.c | 72 +++++++++++++++-----------
> 1 file changed, 43 insertions(+), 29 deletions(-)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c
> b/src/gallium/auxiliary/tgsi/tgsi_scan.c
> index 6e2e23822c..d56844bdc2 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c
> @@ -1028,11 +1028,12 @@ get_block_tessfactor_writemask(const struct
> tgsi_shader_info *info,
> struct tgsi_full_instruction *inst;
> unsigned writemask = 0;
>
> - do {
> - tgsi_parse_token(parse);
> - assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION);
> - inst = &parse->FullToken.FullInstruction;
> - check_no_subroutines(inst);
> + tgsi_parse_token(parse);
> + assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION);
> + inst = &parse->FullToken.FullInstruction;
> + check_no_subroutines(inst);
> +
> + while (inst->Instruction.Opcode != end_opcode) {
>
> /* Recursively process nested blocks. */
> switch (inst->Instruction.Opcode) {
> @@ -1040,20 +1041,26 @@ get_block_tessfactor_writemask(const struct
> tgsi_shader_info *info,
> case TGSI_OPCODE_UIF:
> writemask |=
> get_block_tessfactor_writemask(info, parse,
> TGSI_OPCODE_ENDIF);
> - continue;
> + break;
>
> case TGSI_OPCODE_BGNLOOP:
> writemask |=
> get_block_tessfactor_writemask(info, parse,
> TGSI_OPCODE_ENDLOOP);
> - continue;
> + break;
>
> case TGSI_OPCODE_BARRIER:
> unreachable("nested BARRIER is illegal");
> - continue;
> + break;
> +
> + default:
> + writemask |= get_inst_tessfactor_writemask(info, inst);
> }
>
> - writemask |= get_inst_tessfactor_writemask(info, inst);
> - } while (inst->Instruction.Opcode != end_opcode);
> + tgsi_parse_token(parse);
> + assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION);
> + inst = &parse->FullToken.FullInstruction;
> + check_no_subroutines(inst);
> + }
>
> return writemask;
> }
> @@ -1067,18 +1074,20 @@ get_if_block_tessfactor_writemask(const struct
> tgsi_shader_info *info,
> struct tgsi_full_instruction *inst;
> unsigned then_tessfactor_writemask = 0;
> unsigned else_tessfactor_writemask = 0;
> + unsigned writemask;
> bool is_then = true;
>
> - do {
> - tgsi_parse_token(parse);
> - assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION);
> - inst = &parse->FullToken.FullInstruction;
> - check_no_subroutines(inst);
> + tgsi_parse_token(parse);
> + assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION);
> + inst = &parse->FullToken.FullInstruction;
> + check_no_subroutines(inst);
> +
> + while (inst->Instruction.Opcode != TGSI_OPCODE_ENDIF) {
>
> switch (inst->Instruction.Opcode) {
> case TGSI_OPCODE_ELSE:
> is_then = false;
> - continue;
> + break;
>
> /* Recursively process nested blocks. */
> case TGSI_OPCODE_IF:
> @@ -1087,28 +1096,33 @@ get_if_block_tessfactor_writemask(const struct
> tgsi_shader_info *info,
> is_then ?
> &then_tessfactor_writemask :
>
> &else_tessfactor_writemask,
> cond_block_tf_writemask);
> - continue;
> + break;
>
> case TGSI_OPCODE_BGNLOOP:
> *cond_block_tf_writemask |=
> get_block_tessfactor_writemask(info, parse,
> TGSI_OPCODE_ENDLOOP);
> - continue;
> + break;
>
> case TGSI_OPCODE_BARRIER:
> unreachable("nested BARRIER is illegal");
> - continue;
> - }
> -
> - /* Process an instruction in the current block. */
> - unsigned writemask = get_inst_tessfactor_writemask(info, inst);
> + break;
> + default:
> + /* Process an instruction in the current block. */
> + writemask = get_inst_tessfactor_writemask(info, inst);
>
> - if (writemask) {
> - if (is_then)
> - then_tessfactor_writemask |= writemask;
> - else
> - else_tessfactor_writemask |= writemask;
> + if (writemask) {
> + if (is_then)
> + then_tessfactor_writemask |= writemask;
> + else
> + else_tessfactor_writemask |= writemask;
> + }
> }
> - } while (inst->Instruction.Opcode != TGSI_OPCODE_ENDIF);
> +
> + tgsi_parse_token(parse);
> + assert(parse->FullToken.Token.Type == TGSI_TOKEN_TYPE_INSTRUCTION);
> + inst = &parse->FullToken.FullInstruction;
> + check_no_subroutines(inst);
> + }
>
> if (then_tessfactor_writemask || else_tessfactor_writemask) {
> /* If both statements write the same tess factor channels,
> --
> 2.19.2
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
