[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] pkl: fix codegen for EXCOND containing break/continue
From: |
Jose E. Marchesi |
Subject: |
Re: [PATCH] pkl: fix codegen for EXCOND containing break/continue |
Date: |
Fri, 26 Apr 2024 12:11:55 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Hi Mohammad.
Thanks for the patch.
OK for master.
> The following Poke snippets no longer crash the compiler:
>
> Sample 1:
>
> whlie (condition)
> { break; } ?! E_elem;
>
> Sample 2:
>
> whlie (condition)
> { continue; } ?! E_elem;
>
> 2024-04-26 Mohammad-Reza Nabipoor <mnabipoor@gnu.org>
>
> * libpoke/pkl-ast.h (PKL_AST_BREAK_CONTINUE_STMT_NPOPES): New
> macro.
> (struct pkl_ast_break_continue_stmt): New field `npopes' to keep
> track of number of `pope' instructions.
> * libpoke/pkl-trans.h (struct pkl_trans_escapable_ctx): Add new
> field `npopes'.
> * libpoke/pkl-trans.c (pkl_trans1_pr_comp_stmt): For EXCOND
> expression statements, only keep track of number of `pope' insns.
> (pkl_trans1_ps_comp_stmt): Likewise.
> (pkl_trans1_ps_break_continue_stmt): Update the node with the number
> of `pope' insns.
> * libpoke/pkl-gen.c (pkl_gen_ps_break_continue_stmt): Emit `pope'
> instruction(s).
> (pkl_gen_pr_op_excond): Push the failure result after evaluation
> of the EXCOND's LHS.
> * testsuite/poke.pkl/excond-6.pk: New test.
> * testsuite/poke.pkl/excond-7.pk: Likewise.
> * testsuite/poke.pkl/excond-8.pk: Likewise.
> * testsuite/Makefile.am (EXTRA_DIST): Update.
> ---
> ChangeLog | 22 ++++++++++++++++
> libpoke/pkl-ast.h | 2 ++
> libpoke/pkl-gen.c | 17 ++++++------
> libpoke/pkl-trans.c | 48 ++++++++++++++++++----------------
> libpoke/pkl-trans.h | 1 +
> testsuite/Makefile.am | 3 +++
> testsuite/poke.pkl/excond-6.pk | 20 ++++++++++++++
> testsuite/poke.pkl/excond-7.pk | 7 +++++
> testsuite/poke.pkl/excond-8.pk | 16 ++++++++++++
> 9 files changed, 106 insertions(+), 30 deletions(-)
> create mode 100644 testsuite/poke.pkl/excond-6.pk
> create mode 100644 testsuite/poke.pkl/excond-7.pk
> create mode 100644 testsuite/poke.pkl/excond-8.pk
>
> diff --git a/ChangeLog b/ChangeLog
> index 014233ce..97f4081a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,25 @@
> +2024-04-26 Mohammad-Reza Nabipoor <mnabipoor@gnu.org>
> +
> + * libpoke/pkl-ast.h (PKL_AST_BREAK_CONTINUE_STMT_NPOPES): New
> + macro.
> + (struct pkl_ast_break_continue_stmt): New field `npopes' to keep
> + track of number of `pope' instructions.
> + * libpoke/pkl-trans.h (struct pkl_trans_escapable_ctx): Add new
> + field `npopes'.
> + * libpoke/pkl-trans.c (pkl_trans1_pr_comp_stmt): For EXCOND
> + expression statements, only keep track of number of `pope' insns.
> + (pkl_trans1_ps_comp_stmt): Likewise.
> + (pkl_trans1_ps_break_continue_stmt): Update the node with the number
> + of `pope' insns.
> + * libpoke/pkl-gen.c (pkl_gen_ps_break_continue_stmt): Emit `pope'
> + instruction(s).
> + (pkl_gen_pr_op_excond): Push the failure result after evaluation
> + of the EXCOND's LHS.
> + * testsuite/poke.pkl/excond-6.pk: New test.
> + * testsuite/poke.pkl/excond-7.pk: Likewise.
> + * testsuite/poke.pkl/excond-8.pk: Likewise.
> + * testsuite/Makefile.am (EXTRA_DIST): Update.
> +
> 2024-04-09 Mohammad-Reza Nabipoor <mnabipoor@gnu.org>
>
> * libpoke/pkl-diag.c (pkl_detailed_location): Handle the failure
> diff --git a/libpoke/pkl-ast.h b/libpoke/pkl-ast.h
> index 62bc39e7..1b1a25fd 100644
> --- a/libpoke/pkl-ast.h
> +++ b/libpoke/pkl-ast.h
> @@ -2022,6 +2022,7 @@ pkl_ast_node pkl_ast_make_print_stmt (pkl_ast ast,
> #define PKL_AST_BREAK_CONTINUE_STMT_KIND(AST)
> ((AST)->break_continue_stmt.kind)
> #define PKL_AST_BREAK_CONTINUE_STMT_ENTITY(AST)
> ((AST)->break_continue_stmt.entity)
> #define PKL_AST_BREAK_CONTINUE_STMT_NFRAMES(AST)
> ((AST)->break_continue_stmt.nframes)
> +#define PKL_AST_BREAK_CONTINUE_STMT_NPOPES(AST)
> ((AST)->break_continue_stmt.npopes)
>
> #define PKL_AST_BREAK_CONTINUE_STMT_KIND_BREAK 0
> #define PKL_AST_BREAK_CONTINUE_STMT_KIND_CONTINUE 1
> @@ -2032,6 +2033,7 @@ struct pkl_ast_break_continue_stmt
> union pkl_ast_node *entity;
> int kind;
> int nframes;
> + int npopes;
> };
>
> pkl_ast_node pkl_ast_make_break_continue_stmt (pkl_ast ast,
> diff --git a/libpoke/pkl-gen.c b/libpoke/pkl-gen.c
> index a0c3ba6f..a17b4f87 100644
> --- a/libpoke/pkl-gen.c
> +++ b/libpoke/pkl-gen.c
> @@ -1319,9 +1319,12 @@ PKL_PHASE_BEGIN_HANDLER
> (pkl_gen_ps_break_continue_stmt)
> pkl_ast_node stmt = PKL_PASS_NODE;
> int kind = PKL_AST_BREAK_CONTINUE_STMT_KIND (stmt);
> int nframes = PKL_AST_BREAK_CONTINUE_STMT_NFRAMES (PKL_PASS_NODE);
> + int npopes = PKL_AST_BREAK_CONTINUE_STMT_NPOPES (PKL_PASS_NODE);
>
> if (nframes > 0)
> pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_POPF, nframes);
> + for (; npopes; --npopes)
> + pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_POPE);
> pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_BA,
> kind == PKL_AST_BREAK_CONTINUE_STMT_KIND_BREAK
> ? pkl_asm_break_label (PKL_GEN_ASM)
> @@ -4635,10 +4638,6 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_op_excond)
> pvm_program_label exception_handler = pkl_asm_fresh_label (PKL_GEN_ASM);
> pvm_program_label done = pkl_asm_fresh_label (PKL_GEN_ASM);
>
> - /* Push the provisional result of the operation, which is
> - `false'. */
> - pkl_asm_insn (pasm, PKL_INSN_PUSH, pvm_make_int (0, 32));
> -
> /* Install a handler for the exception specified in the second
> operand. */
> PKL_PASS_SUBPASS (op2);
> @@ -4651,14 +4650,16 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_pr_op_excond)
> pkl_asm_insn (pasm, PKL_INSN_DROP);
>
> pkl_asm_insn (pasm, PKL_INSN_POPE);
> +
> + /* Push the result of the operation, which is `false'. */
> + pkl_asm_insn (pasm, PKL_INSN_PUSH, pvm_make_int (0, 32));
> +
> pkl_asm_insn (pasm, PKL_INSN_BA, done);
>
> - /* The exception handler just drops the raised exception and the
> - provisional result `false' and pushes `true' to reflect the
> - exception was raised. */
> + /* The exception handler just drops the raised exception and
> + pushes `true' to reflect the exception was raised. */
> pkl_asm_label (pasm, exception_handler);
> pkl_asm_insn (pasm, PKL_INSN_DROP); /* The exception. */
> - pkl_asm_insn (pasm, PKL_INSN_DROP); /* The provisional result. */
> pkl_asm_insn (pasm, PKL_INSN_PUSH, pvm_make_int (1, 32));
>
> pkl_asm_label (pasm, done);
> diff --git a/libpoke/pkl-trans.c b/libpoke/pkl-trans.c
> index 73484a51..55a009c8 100644
> --- a/libpoke/pkl-trans.c
> +++ b/libpoke/pkl-trans.c
> @@ -1241,20 +1241,21 @@ PKL_PHASE_END_HANDLER
> PKL_PHASE_BEGIN_HANDLER (pkl_trans1_pr_comp_stmt)
> {
> if (PKL_TRANS_FUNCTION)
> - {
> - PKL_TRANS_FUNCTION->back++;
> -
> - if (PKL_PASS_PARENT
> - && PKL_AST_CODE (PKL_PASS_PARENT) == PKL_AST_EXP
> - && PKL_AST_EXP_CODE (PKL_PASS_PARENT) == PKL_AST_OP_EXCOND)
> - {
> - PKL_TRANS_FUNCTION->ndrops++;
> - PKL_TRANS_FUNCTION->npopes++;
> - }
> - }
> + PKL_TRANS_FUNCTION->back++;
>
> if (PKL_TRANS_ESCAPABLE)
> PKL_TRANS_ESCAPABLE->nframes++;
> +
> + if (PKL_PASS_PARENT
> + && PKL_AST_CODE (PKL_PASS_PARENT) == PKL_AST_EXP
> + && PKL_AST_EXP_CODE (PKL_PASS_PARENT) == PKL_AST_OP_EXCOND)
> + {
> + if (PKL_TRANS_FUNCTION)
> + PKL_TRANS_FUNCTION->npopes++;
> +
> + if (PKL_TRANS_ESCAPABLE)
> + PKL_TRANS_ESCAPABLE->npopes++;
> + }
> }
> PKL_PHASE_END_HANDLER
>
> @@ -1341,20 +1342,21 @@ PKL_PHASE_BEGIN_HANDLER (pkl_trans1_ps_comp_stmt)
> PKL_AST_COMP_STMT_NUMVARS (comp_stmt) = numvars;
>
> if (PKL_TRANS_FUNCTION)
> - {
> - PKL_TRANS_FUNCTION->back--;
> -
> - if (PKL_PASS_PARENT
> - && PKL_AST_CODE (PKL_PASS_PARENT) == PKL_AST_EXP
> - && PKL_AST_EXP_CODE (PKL_PASS_PARENT) == PKL_AST_OP_EXCOND)
> - {
> - PKL_TRANS_FUNCTION->ndrops--;
> - PKL_TRANS_FUNCTION->npopes--;
> - }
> - }
> + PKL_TRANS_FUNCTION->back--;
>
> if (PKL_TRANS_ESCAPABLE)
> PKL_TRANS_ESCAPABLE->nframes--;
> +
> + if (PKL_PASS_PARENT
> + && PKL_AST_CODE (PKL_PASS_PARENT) == PKL_AST_EXP
> + && PKL_AST_EXP_CODE (PKL_PASS_PARENT) == PKL_AST_OP_EXCOND)
> + {
> + if (PKL_TRANS_FUNCTION)
> + PKL_TRANS_FUNCTION->npopes--;
> +
> + if (PKL_TRANS_ESCAPABLE)
> + PKL_TRANS_ESCAPABLE->npopes--;
> + }
> }
> PKL_PHASE_END_HANDLER
>
> @@ -1487,6 +1489,8 @@ PKL_PHASE_BEGIN_HANDLER
> (pkl_trans1_ps_break_continue_stmt)
> = PKL_TRANS_ESCAPABLE->node;
> PKL_AST_BREAK_CONTINUE_STMT_NFRAMES (PKL_PASS_NODE)
> = PKL_TRANS_ESCAPABLE->nframes;
> + PKL_AST_BREAK_CONTINUE_STMT_NPOPES (PKL_PASS_NODE)
> + = PKL_TRANS_ESCAPABLE->npopes;
> }
> }
> PKL_PHASE_END_HANDLER
> diff --git a/libpoke/pkl-trans.h b/libpoke/pkl-trans.h
> index 5379284f..e67862bd 100644
> --- a/libpoke/pkl-trans.h
> +++ b/libpoke/pkl-trans.h
> @@ -66,6 +66,7 @@ struct pkl_trans_escapable_ctx
> {
> pkl_ast_node node;
> int nframes;
> + int npopes;
> };
>
> /* The following struct defines the payload of the trans phases.
> diff --git a/testsuite/Makefile.am b/testsuite/Makefile.am
> index cc212167..6cb09908 100644
> --- a/testsuite/Makefile.am
> +++ b/testsuite/Makefile.am
> @@ -1412,6 +1412,9 @@ EXTRA_DIST = \
> poke.pkl/excond-3.pk \
> poke.pkl/excond-4.pk \
> poke.pkl/excond-5.pk \
> + poke.pkl/excond-6.pk \
> + poke.pkl/excond-7.pk \
> + poke.pkl/excond-8.pk \
> poke.pkl/excond-diag-1.pk \
> poke.pkl/field-init-1.pk \
> poke.pkl/field-init-2.pk \
> diff --git a/testsuite/poke.pkl/excond-6.pk b/testsuite/poke.pkl/excond-6.pk
> new file mode 100644
> index 00000000..3f588284
> --- /dev/null
> +++ b/testsuite/poke.pkl/excond-6.pk
> @@ -0,0 +1,20 @@
> +/* { dg-do run } */
> +
> +fun f = int:
> + {
> + var i = 0;
> +
> + if ({
> + while (1)
> + break;
> + while (0)
> + continue;
> + i = lambda int: { return 1; } ();
> + } ?! E_elem)
> + return 2;
> +
> + return i;
> + }
> +
> +/* { dg-command { f } } */
> +/* { dg-output "1" } */
> diff --git a/testsuite/poke.pkl/excond-7.pk b/testsuite/poke.pkl/excond-7.pk
> new file mode 100644
> index 00000000..a966fab5
> --- /dev/null
> +++ b/testsuite/poke.pkl/excond-7.pk
> @@ -0,0 +1,7 @@
> +/* { dg-do run } */
> +
> +while (1)
> + { break; } ?! E_elem;
> +
> +/* { dg-command {1} } */
> +/* { dg-output "1" } */
> diff --git a/testsuite/poke.pkl/excond-8.pk b/testsuite/poke.pkl/excond-8.pk
> new file mode 100644
> index 00000000..9cbeae16
> --- /dev/null
> +++ b/testsuite/poke.pkl/excond-8.pk
> @@ -0,0 +1,16 @@
> +/* { dg-do run } */
> +
> +var i = 0;
> +while (1)
> + {
> + i++;
> + if (i == 1000_000)
> + break;
> + if ({ continue; } ?! E_elem)
> + assert (0, "first unreachable reached!");
> + else
> + assert (0, "second unreachable reached!");
> + }
> +
> +/* { dg-command {i} } */
> +/* { dg-output "1000000" } */