[v2,14/22] target-mips: add Addressing and PC-relative instructions

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 disas/mips.c            |   42 +++++++++-
 target-mips/translate.c |  198 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 227 insertions(+), 13 deletions(-)

The patch subject is a bit misleading, as it also includes the AUI family.


On Wed, Jun 11, 2014 at 04:19:44PM +0100, Leon Alrae wrote:
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
>  disas/mips.c            |   42 +++++++++-
>  target-mips/translate.c |  198 ++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 227 insertions(+), 13 deletions(-)
> 
> diff --git a/disas/mips.c b/disas/mips.c
> index bee39d8..e041858 100644
> --- a/disas/mips.c
> +++ b/disas/mips.c
> @@ -407,6 +407,12 @@ struct mips_opcode
>     "+3" UDI immediate bits 6-20
>     "+4" UDI immediate bits 6-25
>  
> +   R6 immediates/displacements :
> +   (adding suffix to 'o' to avoid adding new characters)
> +   "+o"  9 bits immediate/displacement (shift = 7)
> +   "+o1" 18 bits immediate/displacement (shift = 0)
> +   "+o2" 19 bits immediate/displacement (shift = 0)
> +
>     Other:
>     "()" parens surrounding optional value
>     ","  separates operands
> @@ -1217,6 +1223,17 @@ const struct mips_opcode mips_builtin_opcodes[] =
>     them first.  The assemblers uses a hash table based on the
>     instruction name anyhow.  */
>  /* name,    args,	match,	    mask,	pinfo,          	membership */
> +{"lwpc",    "s,+o2",    0xec080000, 0xfc180000, WR_d,                 0, I32R6},
> +{"lwupc",   "s,+o2",    0xec100000, 0xfc180000, WR_d,                 0, I32R6},
> +{"ldpc",    "s,+o1",    0xec180000, 0xfc1a0000, WR_d,                 0, I32R6},
> +{"addiupc", "s,+o2",    0xec000000, 0xfc180000, WR_d,                 0, I32R6},
> +{"auipc",   "s,u",      0xec1e0000, 0xfc1f0000, WR_d,                 0, I32R6},
> +{"aluipc",  "s,u",      0xec1f0000, 0xfc1f0000, WR_d,                 0, I32R6},
> +{"daui",    "s,t,u",    0x74000000, 0xfc000000, RD_s|WR_t,            0, I64R6},
> +{"dahi",    "s,u",      0x04060000, 0xfc1f0000, RD_s,                 0, I64R6},
> +{"dati",    "s,u",      0x041e0000, 0xfc1f0000, RD_s,                 0, I64R6},
> +{"lsa",     "d,s,t",    0x00000005, 0xfc00073f, WR_d|RD_s|RD_t,       0, I32R6},
> +{"dlsa",    "d,s,t",    0x00000015, 0xfc00073f, WR_d|RD_s|RD_t,       0, I64R6},
>  {"clz",     "U,s",      0x00000050, 0xfc0007ff, WR_d|RD_s,            0, I32R6},
>  {"clo",     "U,s",      0x00000051, 0xfc0007ff, WR_d|RD_s,            0, I32R6},
>  {"dclz",    "U,s",      0x00000052, 0xfc0007ff, WR_d|RD_s,            0, I64R6},
> @@ -1822,6 +1839,7 @@ const struct mips_opcode mips_builtin_opcodes[] =
>  {"lld",	    "t,o(b)",	0xd0000000, 0xfc000000, LDD|RD_b|WR_t,		0,		I3	},
>  {"lld",     "t,A(b)",	0,    (int) M_LLD_AB,	INSN_MACRO,		0,		I3	},
>  {"lui",     "t,u",	0x3c000000, 0xffe00000,	WR_t,			0,		I1	},
> +{"aui",     "s,t,u",    0x3c000000, 0xfc000000, RD_s|WR_t,            0, I32R6},
>  {"luxc1",   "D,t(b)",	0x4c000005, 0xfc00f83f, LDD|WR_D|RD_t|RD_b|FP_D, 0,		I5|I33|N55},
>  {"lw",      "t,o(b)",	0x8c000000, 0xfc000000,	LDD|RD_b|WR_t,		0,		I1	},
>  {"lw",      "t,A(b)",	0,    (int) M_LW_AB,	INSN_MACRO,		0,		I1	},
> @@ -3645,10 +3663,28 @@ print_insn_args (const char *d,
>  	      break;
>  
>              case 'o':
> -                delta = (l >> OP_SH_DELTA_R6) & OP_MASK_DELTA_R6;
> -                if (delta & 0x8000) {
> -                    delta |= ~0xffff;
> +                switch (*(d+1)) {
> +                case '1':
> +                    d++;
> +                    delta = l & ((1 << 18) - 1);
> +                    if (delta & 0x20000) {
> +                        delta |= ~0x1ffff;
> +                    }
> +                    break;
> +                case '2':
> +                    d++;
> +                    delta = l & ((1 << 19) - 1);
> +                    if (delta & 0x40000) {
> +                        delta |= ~0x3ffff;
> +                    }
> +                    break;
> +                default:
> +                    delta = (l >> OP_SH_DELTA_R6) & OP_MASK_DELTA_R6;
> +                    if (delta & 0x8000) {
> +                        delta |= ~0xffff;
> +                    }
>                  }
> +
>                  (*info->fprintf_func) (info->stream, "%d", delta);
>                  break;
>  
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 1226f97..a3cbe48 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -71,6 +71,7 @@ enum {
>      OPC_BGTZ     = (0x07 << 26),
>      OPC_BGTZL    = (0x17 << 26),
>      OPC_JALX     = (0x1D << 26),  /* MIPS 16 only */
> +    OPC_DAUI     = (0x1D << 26),
>      OPC_JALXS    = OPC_JALX | 0x5,
>      /* Load and stores */
>      OPC_LDL      = (0x1A << 26),
> @@ -137,8 +138,25 @@ enum {
>      /* Cache and prefetch */
>      OPC_CACHE    = (0x2F << 26),
>      OPC_PREF     = (0x33 << 26),
> -    /* Reserved major opcode */
> -    OPC_MAJOR3B_RESERVED = (0x3B << 26),
> +    /* PC-relative address computation / loads */
> +    OPC_PCREL    = (0x3B << 26),
> +};
> +
> +/* PC-relative address computation / loads  */
> +#define MASK_OPC_PCREL_TOP2BITS(op)  (MASK_OP_MAJOR(op) | (op & (3 << 19)))
> +#define MASK_OPC_PCREL_TOP5BITS(op)  (MASK_OP_MAJOR(op) | (op & (0x1f << 16)))
> +enum {
> +    /* Instructions determined by bits 19 and 20 */
> +    OPC_ADDIUPC = OPC_PCREL | (0 << 19),
> +    R6_OPC_LWPC = OPC_PCREL | (1 << 19),
> +    OPC_LWUPC   = OPC_PCREL | (2 << 19),
> +
> +    /* Instructions determined by bits 16 ... 20 */
> +    OPC_AUIPC   = OPC_PCREL | (0x1e << 16),
> +    OPC_ALUIPC  = OPC_PCREL | (0x1f << 16),
> +
> +    /* Other */
> +    R6_OPC_LDPC = OPC_PCREL | (6 << 18),
>  };
>  
>  /* MIPS special opcodes */
> @@ -264,6 +282,9 @@ enum {
>      R6_OPC_DCLZ     = 0x12 | OPC_SPECIAL,
>      R6_OPC_DCLO     = 0x13 | OPC_SPECIAL,
>      R6_OPC_SDBBP    = 0x0e | OPC_SPECIAL,
> +
> +    OPC_LSA  = 0x05 | OPC_SPECIAL,
> +    OPC_DLSA = 0x15 | OPC_SPECIAL,
>  };
>  
>  /* Multiplication variants of the vr54xx. */
> @@ -307,6 +328,9 @@ enum {
>      OPC_TEQI     = (0x0C << 16) | OPC_REGIMM,
>      OPC_TNEI     = (0x0E << 16) | OPC_REGIMM,
>      OPC_SYNCI    = (0x1F << 16) | OPC_REGIMM,
> +
> +    OPC_DAHI     = (0x06 << 16) | OPC_REGIMM,
> +    OPC_DATI     = (0x1e << 16) | OPC_REGIMM,
>  };
>  
>  /* Special2 opcodes */
> @@ -2158,8 +2182,15 @@ static void gen_logic_imm(DisasContext *ctx, uint32_t opc,
>                     regnames[rs], uimm);
>          break;
>      case OPC_LUI:
> -        tcg_gen_movi_tl(cpu_gpr[rt], imm << 16);
> -        MIPS_DEBUG("lui %s, " TARGET_FMT_lx, regnames[rt], uimm);
> +        if (rs != 0 && ctx->insn_flags & ISA_MIPS32R6) {
> +            /* OPC_AUI */
> +            tcg_gen_addi_tl(cpu_gpr[rt], cpu_gpr[rs], imm << 16);
> +            tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]);
> +            MIPS_DEBUG("aui %s, %s, %04x", regnames[rt], regnames[rs], imm);
> +        } else {
> +            tcg_gen_movi_tl(cpu_gpr[rt], imm << 16);
> +            MIPS_DEBUG("lui %s, " TARGET_FMT_lx, regnames[rt], uimm);
> +        }
>          break;
>  
>      default:
> @@ -2763,6 +2794,77 @@ static void gen_HILO(DisasContext *ctx, uint32_t opc, int acc, int reg)
>      MIPS_DEBUG("%s %s", opn, regnames[reg]);
>  }
>  
> +static inline void gen_r6_ld(target_long addr, int reg, int memidx,
> +                             TCGMemOp memop)
> +{
> +    TCGv t0 = tcg_const_tl(addr);
> +    tcg_gen_qemu_ld_tl(t0, t0, memidx, memop);
> +    gen_store_gpr(t0, reg);
> +    tcg_temp_free(t0);
> +}
> +
> +static inline void gen_pcrel(DisasContext *ctx, int rs, int16_t imm)
> +{
> +    target_long offset;
> +    target_long addr;
> +
> +    switch (MASK_OPC_PCREL_TOP2BITS(ctx->opcode)) {
> +    case OPC_ADDIUPC:
> +        if (rs != 0) {
> +            offset = ((int32_t)ctx->opcode << 13) >> 11;
> +            addr = addr_add(ctx, ctx->pc, offset);
> +            tcg_gen_movi_tl(cpu_gpr[rs], addr);
> +        }
> +        break;
> +    case R6_OPC_LWPC:
> +        offset = ((int32_t)ctx->opcode << 13) >> 11;
> +        addr = addr_add(ctx, ctx->pc, offset);
> +        gen_r6_ld(addr, rs, ctx->mem_idx, MO_TESL);
> +        break;
> +#if defined(TARGET_MIPS64)
> +    case OPC_LWUPC:
> +        check_mips_64(ctx);
> +        offset = ((int32_t)ctx->opcode << 13) >> 11;
> +        addr = addr_add(ctx, ctx->pc, offset);
> +        gen_r6_ld(addr, rs, ctx->mem_idx, MO_TEUL);
> +        break;
> +#endif
> +    default:
> +        switch (MASK_OPC_PCREL_TOP5BITS(ctx->opcode)) {
> +        case OPC_AUIPC:
> +            if (rs != 0) {
> +                offset = imm << 16;
> +                addr = addr_add(ctx, ctx->pc, offset);
> +                tcg_gen_movi_tl(cpu_gpr[rs], addr);
> +            }
> +            break;
> +        case OPC_ALUIPC:
> +            if (rs != 0) {
> +                offset = imm << 16;
> +                addr = ~0xFFFF & addr_add(ctx, ctx->pc, offset);
> +                tcg_gen_movi_tl(cpu_gpr[rs], addr);
> +            }
> +            break;
> +#if defined(TARGET_MIPS64)
> +        case R6_OPC_LDPC: /* bits 18 and 19 are part of immediate */
> +        case R6_OPC_LDPC + (1 << 16):
> +        case R6_OPC_LDPC + (2 << 16):
> +        case R6_OPC_LDPC + (3 << 16):
> +            check_mips_64(ctx);
> +            offset = (((int32_t)ctx->opcode << 14)) >> 11;

This will overflow the 32-bits type. I guess you want:

               offset = (((int32_t)ctx->opcode << 13)) >> 10;

I do wonder if we shouldn't use sextract32() instead of open coding that
now that it is available:

               offset = sextract32(ctx->opcode, 0, 19) << 3;

> +            addr = addr_add(ctx, (ctx->pc & ~0x7), offset);

Why do we need to mask the low 3 bits of the PC? It doesn't appear in
the manual version I have (MD00087 version 6.00).

> +            gen_r6_ld(addr, rs, ctx->mem_idx, MO_TEQ);
> +            break;
> +#endif
> +        default:
> +            MIPS_INVAL("OPC_PCREL");
> +            generate_exception(ctx, EXCP_RI);
> +            break;
> +        }
> +        break;
> +    }
> +}
> +
>  static void gen_r6_muldiv(DisasContext *ctx, int opc, int rd, int rs, int rt)
>  {
>      const char *opn = "r6 mul/div";
> @@ -15052,6 +15154,21 @@ static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx)
>  
>      op1 = MASK_SPECIAL(ctx->opcode);
>      switch (op1) {
> +    case OPC_LSA:
> +        {
> +            int imm2 = (ctx->opcode >> 6) & 0x3;
> +            TCGv t0 = tcg_temp_new();
> +            TCGv t1 = tcg_temp_new();
> +            gen_load_gpr(t0, rs);
> +            gen_load_gpr(t1, rt);
> +            tcg_gen_shli_tl(t0, t0, imm2 + 1);
> +            tcg_gen_add_tl(t0, t0, t1);
> +            tcg_gen_ext32s_tl(t0, t0);
> +            gen_store_gpr(t0, rd);
> +            tcg_temp_free(t1);
> +            tcg_temp_free(t0);
> +        }
> +        break;
>      case OPC_MULT ... OPC_DIVU:
>          op2 = MASK_R6_MULDIV(ctx->opcode);
>          switch (op2) {
> @@ -15089,6 +15206,21 @@ static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx)
>          generate_exception(ctx, EXCP_DBp);
>          break;
>  #if defined(TARGET_MIPS64)
> +    case OPC_DLSA:
> +        check_mips_64(ctx);
> +        {
> +            int imm2 = (ctx->opcode >> 6) & 0x3;
> +            TCGv t0 = tcg_temp_new();
> +            TCGv t1 = tcg_temp_new();
> +            gen_load_gpr(t0, rs);
> +            gen_load_gpr(t1, rt);
> +            tcg_gen_shli_tl(t0, t0, imm2 + 1);
> +            tcg_gen_add_tl(t0, t0, t1);
> +            gen_store_gpr(t0, rd);
> +            tcg_temp_free(t1);
> +            tcg_temp_free(t0);
> +        }
> +        break;
>      case R6_OPC_DCLO:
>      case R6_OPC_DCLZ:
>          if (rt == 0 && sa == 1) {
> @@ -15274,13 +15406,18 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
>      case OPC_TNE:
>          gen_trap(ctx, op1, rs, rt, -1);
>          break;
> -    case OPC_PMON:          /* Pmon entry point, also R4010 selsl */
> +    case OPC_LSA: /* OPC_PMON */
> +        if (ctx->insn_flags & ISA_MIPS32R6) {
> +            decode_opc_special_r6(env, ctx);
> +        } else {
> +            /* Pmon entry point, also R4010 selsl */
>  #ifdef MIPS_STRICT_STANDARD
>          MIPS_INVAL("PMON / selsl");
>          generate_exception(ctx, EXCP_RI);
>  #else
>          gen_helper_0e0i(pmon, sa);
>  #endif
> +        }
>          break;
>      case OPC_SYSCALL:
>          generate_exception(ctx, EXCP_SYSCALL);
> @@ -16251,6 +16388,24 @@ static void decode_opc (CPUMIPSState *env, DisasContext *ctx)
>              check_dsp(ctx);
>              gen_compute_branch(ctx, op1, 4, -1, -2, (int32_t)imm << 2);
>              break;
> +#if defined(TARGET_MIPS64)
> +        case OPC_DAHI:
> +            check_insn(ctx, ISA_MIPS32R6);
> +            check_mips_64(ctx);
> +            if (rs != 0) {
> +                tcg_gen_addi_i64(cpu_gpr[rs], cpu_gpr[rs], (int64_t)imm << 32);

Small nitpicking: even if it is guarded by #ifdef, in theory the _tl
type should be used there, to match the register type.

> +            }
> +            MIPS_DEBUG("dahi %s, %04x", regnames[rs], imm);
> +            break;
> +        case OPC_DATI:
> +            check_insn(ctx, ISA_MIPS32R6);
> +            check_mips_64(ctx);
> +            if (rs != 0) {
> +                tcg_gen_addi_i64(cpu_gpr[rs], cpu_gpr[rs], (int64_t)imm << 48);

Ditto

> +            }
> +            MIPS_DEBUG("dati %s, %04x", regnames[rs], imm);
> +            break;
> +#endif
>          default:            /* Invalid */
>              MIPS_INVAL("regimm");
>              generate_exception(ctx, EXCP_RI);
> @@ -16363,7 +16518,7 @@ static void decode_opc (CPUMIPSState *env, DisasContext *ctx)
>           gen_slt_imm(ctx, op, rt, rs, imm);
>           break;
>      case OPC_ANDI: /* Arithmetic with immediate opcode */
> -    case OPC_LUI:
> +    case OPC_LUI: /* OPC_AUI */
>      case OPC_ORI:
>      case OPC_XORI:
>           gen_logic_imm(ctx, op, rt, rs, imm);
> @@ -16661,14 +16816,37 @@ static void decode_opc (CPUMIPSState *env, DisasContext *ctx)
>          }
>          break;
>  #endif
> -    case OPC_JALX:
> -        check_insn(ctx, ASE_MIPS16 | ASE_MICROMIPS);
> -        offset = (int32_t)(ctx->opcode & 0x3FFFFFF) << 2;
> -        gen_compute_branch(ctx, op, 4, rs, rt, offset);
> +    case OPC_DAUI: /* OPC_JALX */
> +        if (ctx->insn_flags & ISA_MIPS32R6) {
> +#if defined(TARGET_MIPS64)
> +            /* OPC_DAUI */
> +            check_mips_64(ctx);
> +            if (rt != 0) {
> +                TCGv_i64 t0 = tcg_temp_new_i64();
> +                gen_load_gpr(t0, rs);
> +                tcg_gen_addi_i64(cpu_gpr[rt], t0, imm << 16);
> +                tcg_temp_free_i64(t0);

Ditto.

> +            }
> +            MIPS_DEBUG("daui %s, %s, %04x", regnames[rt], regnames[rs], imm);
> +#else
> +            generate_exception(ctx, EXCP_RI);
> +            MIPS_INVAL("major opcode");
> +#endif
> +        } else {
> +            /* OPC_JALX */
> +            check_insn(ctx, ASE_MIPS16 | ASE_MICROMIPS);
> +            offset = (int32_t)(ctx->opcode & 0x3FFFFFF) << 2;
> +            gen_compute_branch(ctx, op, 4, rs, rt, offset);
> +        }
>          break;
>      case OPC_MDMX:
>          check_insn(ctx, ASE_MDMX);
>          /* MDMX: Not implemented. */
> +        break;
> +    case OPC_PCREL:
> +        check_insn(ctx, ISA_MIPS32R6);
> +        gen_pcrel(ctx, rs, imm);
> +        break;
>      default:            /* Invalid */
>          MIPS_INVAL("major opcode");
>          generate_exception(ctx, EXCP_RI);
> -- 
> 1.7.5.4
> 
>

On 20/06/2014 21:50, Aurelien Jarno wrote:
> The patch subject is a bit misleading, as it also includes the AUI family.

Thanks for pointing this out.

>> +#if defined(TARGET_MIPS64)
>> +        case R6_OPC_LDPC: /* bits 18 and 19 are part of immediate */
>> +        case R6_OPC_LDPC + (1 << 16):
>> +        case R6_OPC_LDPC + (2 << 16):
>> +        case R6_OPC_LDPC + (3 << 16):
>> +            check_mips_64(ctx);
>> +            offset = (((int32_t)ctx->opcode << 14)) >> 11;
> 
> This will overflow the 32-bits type. I guess you want:
> 
>                offset = (((int32_t)ctx->opcode << 13)) >> 10;

I think original code is correct (LDPC offset's size is 18 bits so it
won't overflow). However, I just noticed that the comment is misleading
(there should be 'bits 16 and 17' instead of 'bits 18 and 19').

> I do wonder if we shouldn't use sextract32() instead of open coding that
> now that it is available:
> 
>                offset = sextract32(ctx->opcode, 0, 19) << 3;

This looks better, thanks for the suggestion (but since the offset's
size is 18, third argument will be 18, not 19).

>> +            addr = addr_add(ctx, (ctx->pc & ~0x7), offset);
> 
> Why do we need to mask the low 3 bits of the PC? It doesn't appear in
> the manual version I have (MD00087 version 6.00).


It doesn't appear in LDPC pseudo-code, but few lines below there is a
restriction: "LDPC is naturally aligned, by specification".
For load doubleword we need to make the address aligned to 8-byte boundary.

You can also refer to MIPS64 Volume-I (MD00083 version 6.01):
5.1.3.1 PC relative loads (Release 6)
"LDPC: Loads a 64-bit doubleword from a PC relative address, formed by
adding the PC, aligned to 8-bytes by masking off the low 3 bits, to a
sign-extended 18-bit immediate, shifted left by 3 bits, for a 21-bit span."

>> +#if defined(TARGET_MIPS64)
>> +        case OPC_DAHI:
>> +            check_insn(ctx, ISA_MIPS32R6);
>> +            check_mips_64(ctx);
>> +            if (rs != 0) {
>> +                tcg_gen_addi_i64(cpu_gpr[rs], cpu_gpr[rs], (int64_t)imm << 32);
> 
> Small nitpicking: even if it is guarded by #ifdef, in theory the _tl
> type should be used there, to match the register type.

I'll correct it.

Thanks,
Leon

On 24 June 2014 10:50, Leon Alrae <leon.alrae@imgtec.com> wrote:
> On 20/06/2014 21:50, Aurelien Jarno wrote:
>> I do wonder if we shouldn't use sextract32() instead of open coding that
>> now that it is available:
>>
>>                offset = sextract32(ctx->opcode, 0, 19) << 3;
>
> This looks better, thanks for the suggestion (but since the offset's
> size is 18, third argument will be 18, not 19).

This is undefined behaviour in C because of the shift into
the sign bit. Better to shift first and then signextend:

    offset = sextract32(ctx->opcode << 3, 0, 21);

thanks
-- PMM

On 06/24/2014 03:00 AM, Peter Maydell wrote:
> On 24 June 2014 10:50, Leon Alrae <leon.alrae@imgtec.com> wrote:
>> On 20/06/2014 21:50, Aurelien Jarno wrote:
>>> I do wonder if we shouldn't use sextract32() instead of open coding that
>>> now that it is available:
>>>
>>>                offset = sextract32(ctx->opcode, 0, 19) << 3;
>>
>> This looks better, thanks for the suggestion (but since the offset's
>> size is 18, third argument will be 18, not 19).
> 
> This is undefined behaviour in C because of the shift into
> the sign bit. Better to shift first and then signextend:
> 
>     offset = sextract32(ctx->opcode << 3, 0, 21);

Not true.  Because we know from the extract that the value has 13 copies of the
sign bit.  Shifting by 3 isn't going to cause problems.  It's shifting a
*different* bit into the sign position that's (one's compliment) undefined.


r~

PS: Honestly, all these compilers/sanitizers should grow a "No One's
Compliment" switch to disable all the stupid stuff.

On 24 June 2014 15:24, Richard Henderson <rth@twiddle.net> wrote:
> On 06/24/2014 03:00 AM, Peter Maydell wrote:
>> This is undefined behaviour in C because of the shift into
>> the sign bit. Better to shift first and then signextend:
>>
>>     offset = sextract32(ctx->opcode << 3, 0, 21);
>
> Not true.  Because we know from the extract that the value has
> 13 copies of the sign bit.  Shifting by 3 isn't going to cause
> problems.  It's shifting a *different* bit into the sign position
> that's (one's compliment) undefined.

C99 6.5.7 says that for E1 << E2, "If E1 has a signed type
and nonnegative value, and E1 * 2^E2 is representable in
the result type, then that is the resulting value; otherwise,
the behavior is undefined." As I read that, shifting
any negative value is undefined, as well as shifting
a 1 into the sign bit.

> PS: Honestly, all these compilers/sanitizers should grow a "No One's
> Compliment" switch to disable all the stupid stuff.

Heartily agreed. Unfortunately until they do, I don't trust
the compiler not to decide it can be fantastically clever
and speed up specmark by 0.00003% if it breaks the 2s
complement behaviour for signed shifts.

thanks
-- PMM