[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

I have a few questions on the patch implementation:

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.
 * Why not allow the hint syntax above immediate values of 0xf0? Should the
DBG instructions not now be re-implemented as hint aliases?

T2 Hint
 * Same question above for DBG.
 * Why do we still need the InstAlias for the hint here? Can it not be
removed like the T1 version?

The testing looks great. 

Ta
Rich
> -----Original Message-----
> From: Artyom Skrobov
> Sent: 18 October 2013 18:18
> To: Richard Barton; llvm-commits at cs.uiuc.edu
> Subject: [PATCH] Make hint ranges consistent (Was: For the 16-bit
unallocated
> NOP hints, make disassembly representation consistent with their 32-bit
> counterparts)
> 
> Yes; the attached patch includes both the tests for the hint ranges, and
the
> necessary tweaks (in TableGen files and in ARMAsmParser) to make these
> consistent, as described in my previous message.
> 
> Could someone please review these?
> 
> 
> -----Original Message-----
> From: Richard Barton
> Sent: 18 October 2013 15:45
> To: Artyom Skrobov; llvm-commits at cs.uiuc.edu
> Subject: RE: [PATCH] For the 16-bit unallocated NOP hints, make
disassembly
> representation consistent with their 32-bit counterparts
> 
> Hi Artyom
> 
> I have applied your refactored patches as 192972 and 192977.
> 
> So it sounds like the NOP hints are broken in their ARM and wide Thumb
> encodings. Do you plan to supply a patch to fix the issues that you
described?
> 
> Thanks
> Rich
> 
> > -----Original Message-----
> > From: Artyom Skrobov
> > Sent: 18 October 2013 13:10
> > To: Richard Barton; llvm-commits at cs.uiuc.edu
> > Subject: RE: [PATCH] For the 16-bit unallocated NOP hints, make
> disassembly
> > representation consistent with their 32-bit counterparts
> >
> > 1) Attaching the patch split in two, so that someone having the
> > necessary permissions could commit it
> >
> > 2) There are several issues about the range of hints to be supported
> > in
> MC.
> >
> > First of all, given that the tests for out-of-range hints don't exist
> > in
> > Thumb2 nor in ARM (in fact, Joey's r191744 explicitly deleted such
> tests!), it's
> > arguable whether they should go in as part of the "fairly simple
> > change to
> bring
> > Thumb1 in-line with Thumb2 and ARM support for hints". If necessary,
> > they
> can
> > be added in a separate commit.
> >
> > A more important point is that the range of supported hints seems too
> loosely
> > defined. Currently, ARM MC supports HINT #0..#255, out of which the
> > last
> 16
> > overlap with DBG #0..#15 (distinct MCInsts having identical encodings,
> which
> > isn't great). At the same time, Thumb2 MC supports only HINT #0..#7 as
> distinct
> > encodings; the range corresponding to HINT #8..#239 is treated as
> undefined
> > when disassembling, and HINT #8..#255, although accepted by the
> > assembler, are wrapped-around to #0..#7 (distinct MCInsts having
> > identical encodings again). I guess this needs some fixing, e.g. by
> > defining the valid range
> to be
> > #0..#239, so we can have a consistent one-to-one correspondence
> > between MCInsts and encodings. Then we can add
> > (back) some meaningful tests for this range.
> >
> > As far as 16-bit hints are concerned, the range encodable in a narrow
> tHINT is
> > #0..#15, so if we want HINT #16 onwards to assemble in Thumb to the
> > wide t2HINT, rather than to produce an error, it would require even
> > more
> tinkering
> > with the implementation.