Re: [Mesa-dev] [PATCH] nir/spirv: handle if's with same label in both branches
On 09/07/2017 07:03 PM, Jason Ekstrand wrote:
> On Thu, Aug 24, 2017 at 8:16 AM, Juan A. Suarez Romero
> mailto:jasua...@igalia.com>> wrote:
>
> When a conditional branch has the same labels in the "if" part and
> in the
> "else" part, then we have the same cfg block, and it must be handled
> once.
>
> Fixes:
> dEQP-VK.spirv_assembly.instruction.compute.conditional_branch.same_labels*
>
> dEQP-VK.spirv_assembly.instruction.graphics.conditional_branch.same_labels*
> ---
> src/compiler/spirv/vtn_cfg.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_cfg.c
> b/src/compiler/spirv/vtn_cfg.c
> index 03c452cb31..bfca7043cc 100644
> --- a/src/compiler/spirv/vtn_cfg.c
> +++ b/src/compiler/spirv/vtn_cfg.c
> @@ -356,8 +356,11 @@ vtn_cfg_walk_blocks(struct vtn_builder *b,
> struct list_head *cf_list,
> switch_case,
> switch_break,
> loop_break,
> loop_cont);
>
> - if (if_stmt->then_type == vtn_branch_type_none &&
> - if_stmt->else_type == vtn_branch_type_none) {
> + if (then_block == else_block) {
> + block = then_block;
> + continue;
>
>
> This isn't quite sufficient. This needs to be handled the same way as
> OpBranch. In particular,
>
> block->branch_type = if_stmt->then_type;
> if (block->branch_type == vtn_branch_type_none) {
> block = then_block;
> continue;
> } else {
> return;
> }
OK, thanks. I am going to send a v2 soon.
Sam
>
>
> + } else if (if_stmt->then_type == vtn_branch_type_none &&
> + if_stmt->else_type == vtn_branch_type_none) {
> /* Neither side of the if is something we can
> short-circuit. */
> assert((*block->merge & SpvOpCodeMask) ==
> SpvOpSelectionMerge);
> struct vtn_block *merge_block =
> --
> 2.13.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir/spirv: handle if's with same label in both branches
On Thu, Aug 24, 2017 at 8:16 AM, Juan A. Suarez Romero
wrote:
> When a conditional branch has the same labels in the "if" part and in the
> "else" part, then we have the same cfg block, and it must be handled
> once.
>
> Fixes:
> dEQP-VK.spirv_assembly.instruction.compute.conditional_branch.same_labels*
> dEQP-VK.spirv_assembly.instruction.graphics.conditional_branch.same_
> labels*
> ---
> src/compiler/spirv/vtn_cfg.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
> index 03c452cb31..bfca7043cc 100644
> --- a/src/compiler/spirv/vtn_cfg.c
> +++ b/src/compiler/spirv/vtn_cfg.c
> @@ -356,8 +356,11 @@ vtn_cfg_walk_blocks(struct vtn_builder *b, struct
> list_head *cf_list,
>switch_case,
> switch_break,
>loop_break, loop_cont);
>
> - if (if_stmt->then_type == vtn_branch_type_none &&
> - if_stmt->else_type == vtn_branch_type_none) {
> + if (then_block == else_block) {
> +block = then_block;
> +continue;
>
This isn't quite sufficient. This needs to be handled the same way as
OpBranch. In particular,
block->branch_type = if_stmt->then_type;
if (block->branch_type == vtn_branch_type_none) {
block = then_block;
continue;
} else {
return;
}
> + } else if (if_stmt->then_type == vtn_branch_type_none &&
> +if_stmt->else_type == vtn_branch_type_none) {
> /* Neither side of the if is something we can short-circuit.
> */
> assert((*block->merge & SpvOpCodeMask) ==
> SpvOpSelectionMerge);
> struct vtn_block *merge_block =
> --
> 2.13.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir/spirv: handle if's with same label in both branches
This patch is still unreviewed. Gently ping if someone can take a look.
Thanks in advance!
J.A.
On Thu, 2017-08-24 at 17:16 +0200, Juan A. Suarez Romero wrote:
> When a conditional branch has the same labels in the "if" part and in the
> "else" part, then we have the same cfg block, and it must be handled
> once.
>
> Fixes:
> dEQP-VK.spirv_assembly.instruction.compute.conditional_branch.same_labels*
> dEQP-VK.spirv_assembly.instruction.graphics.conditional_branch.same_labels*
> ---
> src/compiler/spirv/vtn_cfg.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
> index 03c452cb31..bfca7043cc 100644
> --- a/src/compiler/spirv/vtn_cfg.c
> +++ b/src/compiler/spirv/vtn_cfg.c
> @@ -356,8 +356,11 @@ vtn_cfg_walk_blocks(struct vtn_builder *b, struct
> list_head *cf_list,
>switch_case, switch_break,
>loop_break, loop_cont);
>
> - if (if_stmt->then_type == vtn_branch_type_none &&
> - if_stmt->else_type == vtn_branch_type_none) {
> + if (then_block == else_block) {
> +block = then_block;
> +continue;
> + } else if (if_stmt->then_type == vtn_branch_type_none &&
> +if_stmt->else_type == vtn_branch_type_none) {
> /* Neither side of the if is something we can short-circuit. */
> assert((*block->merge & SpvOpCodeMask) == SpvOpSelectionMerge);
> struct vtn_block *merge_block =
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir/spirv: handle if's with same label in both branches
When a conditional branch has the same labels in the "if" part and in the
"else" part, then we have the same cfg block, and it must be handled
once.
Fixes:
dEQP-VK.spirv_assembly.instruction.compute.conditional_branch.same_labels*
dEQP-VK.spirv_assembly.instruction.graphics.conditional_branch.same_labels*
---
src/compiler/spirv/vtn_cfg.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
index 03c452cb31..bfca7043cc 100644
--- a/src/compiler/spirv/vtn_cfg.c
+++ b/src/compiler/spirv/vtn_cfg.c
@@ -356,8 +356,11 @@ vtn_cfg_walk_blocks(struct vtn_builder *b, struct
list_head *cf_list,
switch_case, switch_break,
loop_break, loop_cont);
- if (if_stmt->then_type == vtn_branch_type_none &&
- if_stmt->else_type == vtn_branch_type_none) {
+ if (then_block == else_block) {
+block = then_block;
+continue;
+ } else if (if_stmt->then_type == vtn_branch_type_none &&
+if_stmt->else_type == vtn_branch_type_none) {
/* Neither side of the if is something we can short-circuit. */
assert((*block->merge & SpvOpCodeMask) == SpvOpSelectionMerge);
struct vtn_block *merge_block =
--
2.13.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
