[PATCH] Add support for ARM modified immediate syntax

[Mihail Popa]

Hi Tim.

While not beautiful, it's the least ugly solution.
This can't be handled by custom parsing alone unfortunately.
We'd have to change how the immediate is represented. Currently
in MC the immediate is still a single number. If we just parse this syntax
and keep the internal representation of this kind of operand we are
loosing the ability to differentiate between equivalent yet different
encodings.

For example:

and r0, r1, #1, #0
and r0, r1, #1, #2
...
and r0, r1, #1, #30

are are equivalent but have different encodings. The point is to retain
the exact form that was parsed.

This implies that internally MC needs to be changed to hold the [value,
modifier]
pair rather than the value itself. Isn't this uglier, more difficult and
more risky?

To me it seems more natural to solve an essentially syntactical problem
with a
asm parsing only solution.

The other thing is that I have doubts about the asm parser's ability to
support this,
even via custom parsing methods. In the end what tablegen generates is a
parser
in name only. It's not a real parser. I suspect that a certain amount of
hacking would
be required to support an immediate operand which consists of a pair of
values where
the second element is optional and defaults to zero.

Regarding the duplication, this is a tablegen improvement opportunity. We
should
be able to factor out the definitions common to all members of a
multiclass; but
alas this doesn't work.

Mihai



On Mon, Jul 29, 2013 at 4:31 PM, Tim Northover <t.p.northover at gmail.com>wrote:

> Hi Mihail,
>
> > Please review the attached patch. It adds support for "modified
> immediate"
> > syntax form to the relevant ARM instructions. Basically these immediate
> > fields are represented via a pair of values: one being a base value and
> the
> > other a modifier.
>
> A good goal, but I think this is the wrong approach, for two reasons:
> 1. Copy-pasting a dozen instructions usable only for AsmParsing just
> to support this syntax is horrific.
> 2. It doesn't handle (or extend to handling) the disassembly case
> neatly. Some of these instructions *should* print without the extra
> immediate (e.g. "movs r11, #99, #0").
>
> I think this is a perfect candidate for adding a "ParserMethod" to the
> so_imm Operand type (well, SOImmAsmOperand)[1]. Obviously
> isSOImmAsmOperand would have to change too (probably to become
> trivial). Then when you get around to implementing disassembly, you
> just need to add a single PrintMethod and you're done.
>
> Cheers.
>
> Tim.
>
> [1] Well, actually I think your new instructions are better than the
> existing ones and should be canonical, with the single-immediate
> version handled by an InstAlias. Unfortunately, I don't think TableGen
> is anywhere near sophisticated enough to handle that neatly yet)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130729/a83ff352/attachment.html>