Re: [PATCH v13 0/7] MR450: vkd3d-shader/ir: Flatten structured control flow instructions.

[Henri Verbeet (＠hverbeet)]

> +        case VKD3DSIH_BRANCH:
> +        {
> +            const struct vkd3d_shader_src_param *src = ins->src, *merge;
> +
> +            if (ins->src_count > 2)
> +            {
> +                shader_dump_instruction_flags(compiler, ins);
> +                vkd3d_string_buffer_printf(buffer, " ");
> +                shader_dump_src_param(compiler, &src[0]);
> +                shader_print_label_id(compiler, ", ", src[1].reg.idx[0].offset);
> +                shader_print_label_id(compiler, ", ", src[1].reg.idx[1].offset);
> +                merge = &src[2];
> +            }
> +            else
> +            {
> +                shader_print_label_id(compiler, " ", src[0].reg.idx[0].offset);
> +                merge = (ins->src_count > 1) ? &src[1] : NULL;
> +            }
> +            if (merge)
> +            {
> +                shader_print_label_id(compiler, ", ", merge->reg.idx[0].offset);
> +            }
> +            break;
> +        }

Right, but if we used separate parameters for true/false and merge/continue, we could just let the default case handle this instruction. In principle that applies to VKD3DSIH_SWITCH_MONOLITHIC as well.
> +static unsigned int cf_flattener_alloc_block_id(struct cf_flattener *flattener)
> +{
> +    return ++flattener->block_id;
> +}

What happens when there are existing LABEL instructions in the source shader? These aren't terribly common in the wild, and the SPIR-V backend doesn't currently handle CALL/LABEL, but the instructions do exist in both d3dbc and tpf.
> +/* For conditional branches, this returns the true+false target branch parameter . */
> +static struct vkd3d_shader_src_param *cf_flattener_emit_branch(struct cf_flattener *flattener,
> +        unsigned int merge_block_id, const struct vkd3d_shader_src_param *condition,
> +        unsigned int true_id, unsigned int false_id,
> +        unsigned int flags)

Some stray space in the comment above.
> +            case VKD3DSIH_IF:
> +                if (!(cf_info = cf_flattener_push_control_flow_level(flattener)))
> +                    return VKD3D_ERROR_OUT_OF_MEMORY;
> +
> +                true_block_id = cf_flattener_alloc_block_id(flattener);
> +                merge_block_id = cf_flattener_alloc_block_id(flattener);
> +                cf_info->u.if_.targets = cf_flattener_emit_branch(flattener, merge_block_id,
> +                        src, true_block_id, merge_block_id, instruction->flags);
> +                if (!cf_info->u.if_.targets)
> +                    return VKD3D_ERROR_OUT_OF_MEMORY;
> +
> +                cf_flattener_emit_label(flattener, true_block_id);
> +
> +                cf_info->u.if_.id = flattener->branch_id;
> +                cf_info->u.if_.merge_block_id = merge_block_id;
> +                cf_info->u.if_.else_block_id = 0;
> +                cf_info->inside_block = true;
> +                cf_info->current_block = VKD3D_BLOCK_IF;
> +
> +                ++flattener->branch_id;

Technically "branch_id" and "if_.id" aren't used until patch 7/7.