diff mbox

[v14,24/33] target-tilegx: Handle shift instructions

Message ID 1440433079-14458-25-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Aug. 24, 2015, 4:17 p.m. UTC 
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-tilegx/translate.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

Comments

Peter Maydell Aug. 30, 2015, 1:38 p.m. UTC | #1 
On 24 August 2015 at 17:17, Richard Henderson <rth@twiddle.net> wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-tilegx/translate.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
> index 6be751b..4e6d577 100644
> --- a/target-tilegx/translate.c
> +++ b/target-tilegx/translate.c
> @@ -474,6 +474,7 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
>      TCGv tdest = dest_gr(dc, dest);
>      TCGv tsrca = load_gr(dc, srca);
>      TCGv tsrcb = load_gr(dc, srcb);
> +    TCGv t0;

Personally I would restrict the scope of this to just the
case block where it's used.

>      case OE_RRR(SHRUX, 0, X0):
>      case OE_RRR(SHRUX, 0, X1):
> +        t0 = tcg_temp_new();
> +        tcg_gen_andi_tl(t0, tsrcb, 31);
> +        tcg_gen_ext32u_tl(tdest, tsrca);
> +        tcg_gen_shl_tl(tdest, tdest, t0);
> +        tcg_gen_ext32s_tl(tdest, tdest);
> +        tcg_temp_free(t0);
> +        mnemonic = "shrux";
> +        break;

Using shl for a shift right doesn't look right...

>      case OE_SH(SHRUI, X0):
>      case OE_SH(SHRUI, X1):
>      case OE_SH(SHRUI, Y0):
>      case OE_SH(SHRUI, Y1):
> +        tcg_gen_shri_tl(tdest, tsrca, imm);
> +        mnemonic = "shrui";
> +        break;
>      case OE_SH(SHRUXI, X0):
>      case OE_SH(SHRUXI, X1):
> +        if ((imm & 31) == 0) {
> +            tcg_gen_ext32s_tl(tdest, tsrca);
> +        } else {
> +            tcg_gen_ext32u_tl(tdest, tsrca);
> +            tcg_gen_shli_tl(tdest, tdest, imm & 31);

Shift left used when shift right intended ?

> +        }
> +        mnemonic = "shlxi";
> +        break;
>      case OE_SH(V1SHLI, X0):
>      case OE_SH(V1SHLI, X1):
>      case OE_SH(V1SHRSI, X0):
> @@ -1096,6 +1148,7 @@ static TileExcp gen_rri_opcode(DisasContext *dc, unsigned opext,
>      case OE_SH(V2SHRSI, X1):
>      case OE_SH(V2SHRUI, X0):
>      case OE_SH(V2SHRUI, X1):
> +        return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;

Why does this change appear in this patch?

>
>      case OE(ADDLI_OPCODE_X0, 0, X0):
>      case OE(ADDLI_OPCODE_X1, 0, X1):
> --
> 2.4.3
>

thanks
-- PMM
Richard Henderson Sept. 1, 2015, 5:37 a.m. UTC | #2 
On 08/30/2015 06:38 AM, Peter Maydell wrote:
> On 24 August 2015 at 17:17, Richard Henderson <rth@twiddle.net> wrote:
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>>   target-tilegx/translate.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 55 insertions(+), 2 deletions(-)
>>
>> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
>> index 6be751b..4e6d577 100644
>> --- a/target-tilegx/translate.c
>> +++ b/target-tilegx/translate.c
>> @@ -474,6 +474,7 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
>>       TCGv tdest = dest_gr(dc, dest);
>>       TCGv tsrca = load_gr(dc, srca);
>>       TCGv tsrcb = load_gr(dc, srcb);
>> +    TCGv t0;
>
> Personally I would restrict the scope of this to just the
> case block where it's used.

It'll get used lots more by other insns in subsequent patches.  While 
restricting the scope is arguably ideal, it adds 3 lines for each use and 
brevity has its own appeal.

> Using shl for a shift right doesn't look right...
...
> Shift left used when shift right intended ?

Fixed.

>> @@ -1096,6 +1148,7 @@ static TileExcp gen_rri_opcode(DisasContext *dc, unsigned opext,
>>       case OE_SH(V2SHRSI, X1):
>>       case OE_SH(V2SHRUI, X0):
>>       case OE_SH(V2SHRUI, X1):
>> +        return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>
> Why does this change appear in this patch?

I'm sure I meant to rebase it into the patch implementing ADDLI.

>
>>
>>       case OE(ADDLI_OPCODE_X0, 0, X0):
>>       case OE(ADDLI_OPCODE_X1, 0, X1):


r~
diff mbox

Patch

diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index 6be751b..4e6d577 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -474,6 +474,7 @@  static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
     TCGv tdest = dest_gr(dc, dest);
     TCGv tsrca = load_gr(dc, srca);
     TCGv tsrcb = load_gr(dc, srcb);
+    TCGv t0;
     const char *mnemonic;
 
     switch (opext) {
@@ -666,7 +667,10 @@  static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
     case OE_RRR(ROTL, 0, X1):
     case OE_RRR(ROTL, 6, Y0):
     case OE_RRR(ROTL, 6, Y1):
-        return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+        tcg_gen_andi_tl(tdest, tsrcb, 63);
+        tcg_gen_rotl_tl(tdest, tsrca, tdest);
+        mnemonic = "torl";
+        break;
     case OE_RRR(SHL1ADDX, 0, X0):
     case OE_RRR(SHL1ADDX, 0, X1):
     case OE_RRR(SHL1ADDX, 7, Y0):
@@ -720,21 +724,45 @@  static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
         break;
     case OE_RRR(SHLX, 0, X0):
     case OE_RRR(SHLX, 0, X1):
+        tcg_gen_andi_tl(tdest, tsrcb, 31);
+        tcg_gen_shl_tl(tdest, tsrca, tdest);
+        tcg_gen_ext32s_tl(tdest, tdest);
+        mnemonic = "shlx";
+        break;
     case OE_RRR(SHL, 0, X0):
     case OE_RRR(SHL, 0, X1):
     case OE_RRR(SHL, 6, Y0):
     case OE_RRR(SHL, 6, Y1):
+        tcg_gen_andi_tl(tdest, tsrcb, 63);
+        tcg_gen_shl_tl(tdest, tsrca, tdest);
+        mnemonic = "shl";
+        break;
     case OE_RRR(SHRS, 0, X0):
     case OE_RRR(SHRS, 0, X1):
     case OE_RRR(SHRS, 6, Y0):
     case OE_RRR(SHRS, 6, Y1):
+        tcg_gen_andi_tl(tdest, tsrcb, 63);
+        tcg_gen_sar_tl(tdest, tsrca, tdest);
+        mnemonic = "shrs";
+        break;
     case OE_RRR(SHRUX, 0, X0):
     case OE_RRR(SHRUX, 0, X1):
+        t0 = tcg_temp_new();
+        tcg_gen_andi_tl(t0, tsrcb, 31);
+        tcg_gen_ext32u_tl(tdest, tsrca);
+        tcg_gen_shl_tl(tdest, tdest, t0);
+        tcg_gen_ext32s_tl(tdest, tdest);
+        tcg_temp_free(t0);
+        mnemonic = "shrux";
+        break;
     case OE_RRR(SHRU, 0, X0):
     case OE_RRR(SHRU, 0, X1):
     case OE_RRR(SHRU, 6, Y0):
     case OE_RRR(SHRU, 6, Y1):
-        return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+        tcg_gen_andi_tl(tdest, tsrcb, 63);
+        tcg_gen_shr_tl(tdest, tsrca, tdest);
+        mnemonic = "shru";
+        break;
     case OE_RRR(SHUFFLEBYTES, 0, X0):
         gen_helper_shufflebytes(tdest, load_gr(dc, dest), tsrca, tsrca);
         mnemonic = "shufflebytes";
@@ -1068,22 +1096,46 @@  static TileExcp gen_rri_opcode(DisasContext *dc, unsigned opext,
     case OE_SH(ROTLI, X1):
     case OE_SH(ROTLI, Y0):
     case OE_SH(ROTLI, Y1):
+        tcg_gen_rotli_tl(tdest, tsrca, imm);
+        mnemonic = "rotli";
+        break;
     case OE_SH(SHLI, X0):
     case OE_SH(SHLI, X1):
     case OE_SH(SHLI, Y0):
     case OE_SH(SHLI, Y1):
+        tcg_gen_shli_tl(tdest, tsrca, imm);
+        mnemonic = "shli";
+        break;
     case OE_SH(SHLXI, X0):
     case OE_SH(SHLXI, X1):
+        tcg_gen_shli_tl(tdest, tsrca, imm & 31);
+        tcg_gen_ext32s_tl(tdest, tdest);
+        mnemonic = "shlxi";
+        break;
     case OE_SH(SHRSI, X0):
     case OE_SH(SHRSI, X1):
     case OE_SH(SHRSI, Y0):
     case OE_SH(SHRSI, Y1):
+        tcg_gen_sari_tl(tdest, tsrca, imm);
+        mnemonic = "shrsi";
+        break;
     case OE_SH(SHRUI, X0):
     case OE_SH(SHRUI, X1):
     case OE_SH(SHRUI, Y0):
     case OE_SH(SHRUI, Y1):
+        tcg_gen_shri_tl(tdest, tsrca, imm);
+        mnemonic = "shrui";
+        break;
     case OE_SH(SHRUXI, X0):
     case OE_SH(SHRUXI, X1):
+        if ((imm & 31) == 0) {
+            tcg_gen_ext32s_tl(tdest, tsrca);
+        } else {
+            tcg_gen_ext32u_tl(tdest, tsrca);
+            tcg_gen_shli_tl(tdest, tdest, imm & 31);
+        }
+        mnemonic = "shlxi";
+        break;
     case OE_SH(V1SHLI, X0):
     case OE_SH(V1SHLI, X1):
     case OE_SH(V1SHRSI, X0):
@@ -1096,6 +1148,7 @@  static TileExcp gen_rri_opcode(DisasContext *dc, unsigned opext,
     case OE_SH(V2SHRSI, X1):
     case OE_SH(V2SHRUI, X0):
     case OE_SH(V2SHRUI, X1):
+        return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
 
     case OE(ADDLI_OPCODE_X0, 0, X0):
     case OE(ADDLI_OPCODE_X1, 0, X1):