poke-devel
[Top][All Lists]
Advanced

[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" } */



reply via email to

[Prev in Thread] Current Thread [Next in Thread]