[04/20] tcg-ia64: Simplify brcond

There was a misconception that a stop bit is required between a compare
and the branch that uses the predicate set by the compare.  This lead to
the usage of an extra bundle in which to perform the compare.  The extra
bundle left room for constants to be loaded for use with the compare insn.

If we pack the compare and the branch together in the same bundle, then
there's no longer any room for non-zero constants.  At which point we
can eliminate half the function by not handling them.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/ia64/tcg-target.c | 42 +++++++++---------------------------------
 1 file changed, 9 insertions(+), 33 deletions(-)

On Thu, Oct 31, 2013 at 01:21:53PM -0700, Richard Henderson wrote:
> There was a misconception that a stop bit is required between a compare
> and the branch that uses the predicate set by the compare.  This lead to

This seems to be true.

> the usage of an extra bundle in which to perform the compare.  The extra
> bundle left room for constants to be loaded for use with the compare insn.
> 
> If we pack the compare and the branch together in the same bundle, then
> there's no longer any room for non-zero constants.  At which point we
> can eliminate half the function by not handling them.

That said the implementation is likely wrong as with this patch applied,
qemu-system-x86_64 is not even able to execute seabios to the first
printed message. Please do at least basic testing.

> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/ia64/tcg-target.c | 42 +++++++++---------------------------------
>  1 file changed, 9 insertions(+), 33 deletions(-)
> 
> diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c
> index b19e298..2fdc38a5 100644
> --- a/tcg/ia64/tcg-target.c
> +++ b/tcg/ia64/tcg-target.c
> @@ -1444,38 +1444,16 @@ static inline uint64_t tcg_opc_cmp_a(int qp, TCGCond cond, TCGArg arg1,
>      }
>  }
>  
> -static inline void tcg_out_brcond(TCGContext *s, TCGCond cond, TCGArg arg1,
> -                                  int const_arg1, TCGArg arg2, int const_arg2,
> -                                  int label_index, int cmp4)
> +static inline void tcg_out_brcond(TCGContext *s, TCGCond cond, TCGReg arg1,
> +                                  TCGReg arg2, int label_index, int cmp4)
>  {
>      TCGLabel *l = &s->labels[label_index];
> -    uint64_t opc1, opc2;
>  
> -    if (const_arg1 && arg1 != 0) {
> -        opc1 = tcg_opc_a5(TCG_REG_P0, OPC_ADDL_A5, TCG_REG_R2,
> -                          arg1, TCG_REG_R0);
> -        arg1 = TCG_REG_R2;
> -    } else {
> -        opc1 = INSN_NOP_M;
> -    }
> -
> -    if (const_arg2 && arg2 != 0) {
> -        opc2 = tcg_opc_a5(TCG_REG_P0, OPC_ADDL_A5, TCG_REG_R3,
> -                          arg2, TCG_REG_R0);
> -        arg2 = TCG_REG_R3;
> -    } else {
> -        opc2 = INSN_NOP_I;
> -    }
> -
> -    tcg_out_bundle(s, mII,
> -                   opc1,
> -                   opc2,
> -                   tcg_opc_cmp_a(TCG_REG_P0, cond, arg1, arg2, cmp4));
> -    tcg_out_bundle(s, mmB,
> -                   INSN_NOP_M,
> +    tcg_out_bundle(s, miB,
>                     INSN_NOP_M,
> -                   tcg_opc_b1 (TCG_REG_P6, OPC_BR_DPTK_FEW_B1,
> -                               get_reloc_pcrel21b(s->code_ptr + 2)));
> +                   tcg_opc_cmp_a(TCG_REG_P0, cond, arg1, arg2, cmp4),
> +                   tcg_opc_b1(TCG_REG_P6, OPC_BR_DPTK_FEW_B1,
> +                              get_reloc_pcrel21b(s->code_ptr + 2)));
>  
>      if (l->has_value) {
>          reloc_pcrel21b((s->code_ptr - 16) + 2, l->u.value);
> @@ -2224,12 +2202,10 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
>          break;
>  
>      case INDEX_op_brcond_i32:
> -        tcg_out_brcond(s, args[2], args[0], const_args[0],
> -                       args[1], const_args[1], args[3], 1);
> +        tcg_out_brcond(s, args[2], args[0], args[1], args[3], 1);
>          break;
>      case INDEX_op_brcond_i64:
> -        tcg_out_brcond(s, args[2], args[0], const_args[0],
> -                       args[1], const_args[1], args[3], 0);
> +        tcg_out_brcond(s, args[2], args[0], args[1], args[3], 0);
>          break;
>      case INDEX_op_setcond_i32:
>          tcg_out_setcond(s, args[3], args[0], args[1], args[2], 1);
> @@ -2333,7 +2309,7 @@ static const TCGTargetOpDef ia64_op_defs[] = {
>      { INDEX_op_bswap16_i32, { "r", "rZ" } },
>      { INDEX_op_bswap32_i32, { "r", "rZ" } },
>  
> -    { INDEX_op_brcond_i32, { "rI", "rI" } },
> +    { INDEX_op_brcond_i32, { "rZ", "rZ" } },
>      { INDEX_op_setcond_i32, { "r", "rZ", "rZ" } },
>      { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rI", "rI" } },
>  
> -- 
> 1.8.3.1
> 
>

On 11/03/2013 12:56 PM, Aurelien Jarno wrote:
> That said the implementation is likely wrong as with this patch applied,
> qemu-system-x86_64 is not even able to execute seabios to the first
> printed message. Please do at least basic testing.

Really?  I'll look into it.

I do basic testing -- arm, sparc, alpha.  Those are the ones that can
usefully boot via serial console instead of having to set up a tunnel
for a vnc viewer.


r~

On Sun, Nov 03, 2013 at 01:34:12PM -1000, Richard Henderson wrote:
> On 11/03/2013 12:56 PM, Aurelien Jarno wrote:
> > That said the implementation is likely wrong as with this patch applied,
> > qemu-system-x86_64 is not even able to execute seabios to the first
> > printed message. Please do at least basic testing.
> 
> Really?  I'll look into it.
> 
> I do basic testing -- arm, sparc, alpha.  Those are the ones that can
> usefully boot via serial console instead of having to set up a tunnel
> for a vnc viewer.

If you don't want to setup a VNC tunnel, you can also uses -curses
(provide the curses support is compiled) for i386 and x86-64.