[PATCH] Make hint ranges consistent (Was: For the 16-bit unallocated NOP hints, make disassembly representation consistent with their 32-bit counterparts)

[Richard Barton]

Hi Artyom

The additional tests look good, and your explanations are convincing. LGTM.

Do you have commit access?

Ta,
Rich

> -----Original Message-----
> From: Artyom Skrobov
> Sent: 21 October 2013 12:23
> To: Richard Barton; llvm-commits at cs.uiuc.edu
> Subject: RE: [PATCH] Make hint ranges consistent (Was: For the 16-bit
> unallocated NOP hints, make disassembly representation consistent with
> their 32-bit counterparts)
> 
> > A1 Hint:
> > * Should the hint syntax be conditional as well, to match with the
> > defined hint syntaxes? It looks like that whole space is unallocated,
> > so we may as well make it available.
> 
> The hints did support the conditional syntax from the beginning, although
it
> was never tested -- not even in my new tests.
> 
> These further changes on top of my patch should be sufficient:
> 
> diff --git a/test/MC/ARM/basic-arm-instructions.s
> b/test/MC/ARM/basic-arm-instructions.s
> index 3d2562d..aeec633 100644
> --- a/test/MC/ARM/basic-arm-instructions.s
> +++ b/test/MC/ARM/basic-arm-instructions.s
> @@ -2928,7 +2928,7 @@ Lforward:
>          hint #2
>          hint #1
>          hint #0
> -        hint #239
> +        hintgt #239
> 
>  @ CHECK: wfe                            @ encoding: [0x02,0xf0,0x20,0xe3]
>  @ CHECK: wfehi                          @ encoding: [0x02,0xf0,0x20,0x83]
> @@ -2941,4 +2941,4 @@ Lforward:
>  @ CHECK: wfe                            @ encoding: [0x02,0xf0,0x20,0xe3]
>  @ CHECK: yield                          @ encoding: [0x01,0xf0,0x20,0xe3]
>  @ CHECK: nop                            @ encoding: [0x00,0xf0,0x20,0xe3]
> -@ CHECK: hint #239                      @ encoding: [0xef,0xf0,0x20,0xe3]
> +@ CHECK: hintgt #239                    @ encoding: [0xef,0xf0,0x20,0xc3]
> diff --git a/test/MC/ARM/basic-thumb2-instructions.s
> b/test/MC/ARM/basic-thumb2-instructions.s
> index ae88d40..fdb2225 100644
> --- a/test/MC/ARM/basic-thumb2-instructions.s
> +++ b/test/MC/ARM/basic-thumb2-instructions.s
> @@ -3603,9 +3603,10 @@ _func:
>          hint #1
>          hint #0
> 
> -        hint #15
> -        hint #16
> -        hint #239
> +        itet lt
> +        hintlt #15
> +        hintge #16
> +        hintlt #239
> 
>  @ CHECK: wfe                            @ encoding: [0x20,0xbf]
>  @ CHECK: wfi                            @ encoding: [0x30,0xbf]
> @@ -3625,9 +3626,10 @@ _func:
>  @ CHECK: yield                          @ encoding: [0x10,0xbf]
>  @ CHECK: nop                            @ encoding: [0x00,0xbf]
> 
> -@ CHECK: hint #15                       @ encoding: [0xf0,0xbf]
> -@ CHECK: hint.w #16                     @ encoding: [0xaf,0xf3,0x10,0x80]
> -@ CHECK: hint.w #239                    @ encoding: [0xaf,0xf3,0xef,0x80]
> +@ CHECK: itet  lt                       @ encoding: [0xb6,0xbf]
> +@ CHECK: hintlt #15                     @ encoding: [0xf0,0xbf]
> +@ CHECK: hintge.w #16                   @ encoding: [0xaf,0xf3,0x10,0x80]
> +@ CHECK: hintlt.w #239                  @ encoding: [0xaf,0xf3,0xef,0x80]
> 
> 
>
@---------------------------------------------------------------------------
> ---
>  @ Unallocated wide/narrow hints
> 
> 
> > * Why not allow the hint syntax above immediate values of 0xf0? Should
> > the DBG instructions not now be re-implemented as hint aliases?
> 
> It could be done, but it's hard for me to see any benefit from doing so;
and
> it would come at the cost of complicating ARMInstPrinter even further.
> 
> > T2 Hint
> > * Why do we still need the InstAlias for the hint here? Can it not be
> > removed like the T1 version?
> 
> No, the InstAlias proves necessary for picking up the wide hint when the
> value is above #15.
> 
>