[PATCH] For the 16-bit unallocated NOP hints, make disassembly representation consistent with their 32-bit counterparts

[Richard Barton]

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.
> 
> 
> -----Original Message-----
> From: Richard Barton
> Sent: 18 October 2013 10:53
> 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
> 
> This patch looks fine, but two points:
> 
>  * The refactoring in ARMInstPrinter.cpp should go in a separate commit.
>  * There need to be error tests for assembly with out of range immediates
for
> the hints. I think these are missing from the wide hints and the arm hints
too, so
> it would be great to add them here.
> 
> LGTM with those two changed made.
> 
> Regards,
> Rich
> 
> > -----Original Message-----
> > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> > bounces at cs.uiuc.edu] On Behalf Of Artyom Skrobov
> > Sent: 17 October 2013 14:16
> > To: llvm-commits at cs.uiuc.edu
> > Subject: [PATCH] For the 16-bit unallocated NOP hints, make
> > disassembly representation consistent with their 32-bit counterparts
> >
> > Hello,
> >
> > Recently the ARM and Thumb2 backends have been changed to give
> > unallocated memory hints a disassembly form "hint #<immediate>", but
> > the 16-bit unallocated hints have not been similarly changed.
> >
> > The 32-bit hints with defined disassembly (NOP through to SEVL) are
> > implemented as InstAliases to a top level MCInst called HINT (for ARM)
> > or t2HINT (for Thumb2) that holds the "hint" disassembly notation.
> > Thus for
> ARM
> > and Thumb2 "hint #4" can be used to mean "SEV", for example. However,
> > the 16-bit Thumb hints are implemented as their own MCInsts, and the
> > top level MCInst called T1SystemEncoding does not have a generic hint
> definition.
> >
> > We've implemented a fairly simple change to bring Thumb1 in-line with
> Thumb2
> > and ARM support for hints. This also adds support for wide vs. narrow
> > hint
> > notation: assembling "hint #7" gives the 16-bit encoding, with "hint.w
#7"
> > required to get the 32-bit encoding. Although there's no pressing need
> > for
> this,
> > as the UAL and ARMARM don't cover disassembly for these instructions,
> > but
> it
> > makes the hints consistent with the rest of the instruction set.