diff mbox series

[for-7.1,v6,01/51] tcg: Fix indirect lowering vs TCG_OPF_COND_BRANCH

Message ID 20220317050538.924111-2-richard.henderson@linaro.org
State New
Headers show
Series target/nios2: Shadow register set, EIC and VIC | expand

Commit Message

Richard Henderson March 17, 2022, 5:04 a.m. UTC 
With TCG_OPF_COND_BRANCH, we extended the lifetimes of
globals across extended basic blocks.  This means that
the liveness computed in pass 1 does not kill globals
in the same way as normal temps.

Introduce TYPE_EBB to match this lifetime, so that we
get correct register allocation for the temps that we
introduce during the indirect lowering pass.

Fixes: b4cb76e6208 ("tcg: Do not kill globals at conditional branches")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/tcg/tcg.h |  2 ++
 tcg/tcg.c         | 10 ++++++++++
 2 files changed, 12 insertions(+)

Comments

Peter Maydell March 17, 2022, 1:38 p.m. UTC | #1 
On Thu, 17 Mar 2022 at 05:09, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> With TCG_OPF_COND_BRANCH, we extended the lifetimes of
> globals across extended basic blocks.  This means that
> the liveness computed in pass 1 does not kill globals
> in the same way as normal temps.
>
> Introduce TYPE_EBB to match this lifetime, so that we
> get correct register allocation for the temps that we
> introduce during the indirect lowering pass.
>
> Fixes: b4cb76e6208 ("tcg: Do not kill globals at conditional branches")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/tcg/tcg.h |  2 ++
>  tcg/tcg.c         | 10 ++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
> index 73869fd9d0..27de13fae0 100644
> --- a/include/tcg/tcg.h
> +++ b/include/tcg/tcg.h
> @@ -433,6 +433,8 @@ typedef enum TCGTempVal {
>  typedef enum TCGTempKind {
>      /* Temp is dead at the end of all basic blocks. */
>      TEMP_NORMAL,
> +    /* Temp is live across conditional branch, but dead otherwise. */
> +    TEMP_EBB,
>      /* Temp is saved across basic blocks but dead at the end of TBs. */
>      TEMP_LOCAL,
>      /* Temp is saved across both basic blocks and translation blocks. */

Maybe add an assert() in tcg_temp_free_internal() that ts->kind is
not TEMP_EBB ?  (This was about the only place in the file that
does different things based on ts->kind that you haven't added
TEMP_EBB handling for.)

> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 33a97eabdb..45030e88fd 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1674,6 +1674,7 @@ static void tcg_reg_alloc_start(TCGContext *s)
>          case TEMP_GLOBAL:
>              break;
>          case TEMP_NORMAL:
> +        case TEMP_EBB:
>              val = TEMP_VAL_DEAD;
>              /* fall through */
>          case TEMP_LOCAL:
> @@ -1701,6 +1702,9 @@ static char *tcg_get_arg_str_ptr(TCGContext *s, char *buf, int buf_size,
>      case TEMP_LOCAL:
>          snprintf(buf, buf_size, "loc%d", idx - s->nb_globals);
>          break;
> +    case TEMP_EBB:
> +        snprintf(buf, buf_size, "ebb%d", idx - s->nb_globals);
> +        break;
>      case TEMP_NORMAL:
>          snprintf(buf, buf_size, "tmp%d", idx - s->nb_globals);
>          break;
> @@ -2378,6 +2382,7 @@ static void la_bb_end(TCGContext *s, int ng, int nt)
>              state = TS_DEAD | TS_MEM;
>              break;
>          case TEMP_NORMAL:
> +        case TEMP_EBB:
>          case TEMP_CONST:
>              state = TS_DEAD;
>              break;
> @@ -2427,6 +2432,7 @@ static void la_bb_sync(TCGContext *s, int ng, int nt)
>          case TEMP_NORMAL:
>              s->temps[i].state = TS_DEAD;
>              break;
> +        case TEMP_EBB:
>          case TEMP_CONST:
>              continue;
>          default:

The comment on la_bb_sync() needs updating:

/*
 * liveness analysis: conditional branch: all temps are dead
 * unless explicitly live-across-conditional-branch, globals
 * and local temps should be synced.
 */


> @@ -2797,6 +2803,7 @@ static bool liveness_pass_2(TCGContext *s)
>              TCGTemp *dts = tcg_temp_alloc(s);
>              dts->type = its->type;
>              dts->base_type = its->base_type;
> +            dts->kind = TEMP_EBB;
>              its->state_ptr = dts;
>          } else {
>              its->state_ptr = NULL;
> @@ -3107,6 +3114,7 @@ static void temp_free_or_dead(TCGContext *s, TCGTemp *ts, int free_or_dead)
>          new_type = TEMP_VAL_MEM;
>          break;
>      case TEMP_NORMAL:
> +    case TEMP_EBB:
>          new_type = free_or_dead < 0 ? TEMP_VAL_MEM : TEMP_VAL_DEAD;
>          break;
>      case TEMP_CONST:
> @@ -3353,6 +3361,7 @@ static void tcg_reg_alloc_bb_end(TCGContext *s, TCGRegSet allocated_regs)
>              temp_save(s, ts, allocated_regs);
>              break;
>          case TEMP_NORMAL:
> +        case TEMP_EBB:
>              /* The liveness analysis already ensures that temps are dead.
>                 Keep an tcg_debug_assert for safety. */
>              tcg_debug_assert(ts->val_type == TEMP_VAL_DEAD);
> @@ -3390,6 +3399,7 @@ static void tcg_reg_alloc_cbranch(TCGContext *s, TCGRegSet allocated_regs)
>          case TEMP_NORMAL:
>              tcg_debug_assert(ts->val_type == TEMP_VAL_DEAD);
>              break;
> +        case TEMP_EBB:
>          case TEMP_CONST:
>              break;
>          default:

Similarly, the comment above tcg_reg_alloc_cbranch() now should
say "all temporaries are dead unless explicitly
live-across-conditional-branch".

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(though review from somebody more familiar with the TCG internals
than me would still be useful I think)

thanks
-- PMM
diff mbox series

Patch

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 73869fd9d0..27de13fae0 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -433,6 +433,8 @@  typedef enum TCGTempVal {
 typedef enum TCGTempKind {
     /* Temp is dead at the end of all basic blocks. */
     TEMP_NORMAL,
+    /* Temp is live across conditional branch, but dead otherwise. */
+    TEMP_EBB,
     /* Temp is saved across basic blocks but dead at the end of TBs. */
     TEMP_LOCAL,
     /* Temp is saved across both basic blocks and translation blocks. */
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 33a97eabdb..45030e88fd 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1674,6 +1674,7 @@  static void tcg_reg_alloc_start(TCGContext *s)
         case TEMP_GLOBAL:
             break;
         case TEMP_NORMAL:
+        case TEMP_EBB:
             val = TEMP_VAL_DEAD;
             /* fall through */
         case TEMP_LOCAL:
@@ -1701,6 +1702,9 @@  static char *tcg_get_arg_str_ptr(TCGContext *s, char *buf, int buf_size,
     case TEMP_LOCAL:
         snprintf(buf, buf_size, "loc%d", idx - s->nb_globals);
         break;
+    case TEMP_EBB:
+        snprintf(buf, buf_size, "ebb%d", idx - s->nb_globals);
+        break;
     case TEMP_NORMAL:
         snprintf(buf, buf_size, "tmp%d", idx - s->nb_globals);
         break;
@@ -2378,6 +2382,7 @@  static void la_bb_end(TCGContext *s, int ng, int nt)
             state = TS_DEAD | TS_MEM;
             break;
         case TEMP_NORMAL:
+        case TEMP_EBB:
         case TEMP_CONST:
             state = TS_DEAD;
             break;
@@ -2427,6 +2432,7 @@  static void la_bb_sync(TCGContext *s, int ng, int nt)
         case TEMP_NORMAL:
             s->temps[i].state = TS_DEAD;
             break;
+        case TEMP_EBB:
         case TEMP_CONST:
             continue;
         default:
@@ -2797,6 +2803,7 @@  static bool liveness_pass_2(TCGContext *s)
             TCGTemp *dts = tcg_temp_alloc(s);
             dts->type = its->type;
             dts->base_type = its->base_type;
+            dts->kind = TEMP_EBB;
             its->state_ptr = dts;
         } else {
             its->state_ptr = NULL;
@@ -3107,6 +3114,7 @@  static void temp_free_or_dead(TCGContext *s, TCGTemp *ts, int free_or_dead)
         new_type = TEMP_VAL_MEM;
         break;
     case TEMP_NORMAL:
+    case TEMP_EBB:
         new_type = free_or_dead < 0 ? TEMP_VAL_MEM : TEMP_VAL_DEAD;
         break;
     case TEMP_CONST:
@@ -3353,6 +3361,7 @@  static void tcg_reg_alloc_bb_end(TCGContext *s, TCGRegSet allocated_regs)
             temp_save(s, ts, allocated_regs);
             break;
         case TEMP_NORMAL:
+        case TEMP_EBB:
             /* The liveness analysis already ensures that temps are dead.
                Keep an tcg_debug_assert for safety. */
             tcg_debug_assert(ts->val_type == TEMP_VAL_DEAD);
@@ -3390,6 +3399,7 @@  static void tcg_reg_alloc_cbranch(TCGContext *s, TCGRegSet allocated_regs)
         case TEMP_NORMAL:
             tcg_debug_assert(ts->val_type == TEMP_VAL_DEAD);
             break;
+        case TEMP_EBB:
         case TEMP_CONST:
             break;
         default: