[PATCH] Add support for ARM modified immediate syntax

[Mihail Popa]

Tim,

changing the representation of these immediate operands breaks frame
lowering.
There is a lot of code along the lines of:

      AddDefaultPred(BuildMI(MBB, MBBI, dl, TII.get(ARM::tMOVr), ARM::R4)
        .addReg(ARM::SP, RegState::Kill));
      AddDefaultCC(AddDefaultPred(BuildMI(MBB, MBBI, dl,
                                          TII.get(ARM::t2BICri), ARM::R4)
                                  .addReg(ARM::R4, RegState::Kill)
                                  .addImm(MaxAlign-1)));
      AddDefaultPred(BuildMI(MBB, MBBI, dl, TII.get(ARM::tMOVr), ARM::SP)
        .addReg(ARM::R4, RegState::Kill));

There are a number of places in the backend where MachineInstructions are
created this way. Since most
code deals with adds, subs and alignment, most addImm calls should be
changed. addImm calls alone
are around 300. In addition to that, some instructions get modified and
will have to be resized, adding an
additional operand.

The best solution I can think of is to derive a class out of
MachineInstructionBuilder which will provide methods
for creating two operand immediates.  The issue still remains of reviewing
each and every addImm in there.

The effort is not the issue here. Do you really think it's work the risk?
Do you really think this is a better and neater
solution? If you do, I'll do it - but I must say that the original patch I
submitted is the solution that makes more
sense simply because it keeps changes local.

The scope of these changes are simply much beyond the scope of the benefit
to warrant implementation.
However, you decide.  Let me know.

Regards,
Mihai





On Tue, Aug 13, 2013 at 3:25 PM, Tim Northover <tnorthover at apple.com> wrote:

> On 13 Aug 2013, at 14:36, Mihail Popa <mihail.popa at gmail.com> wrote:
> > Is there any proper documentation for what ComplexPatterns do and how
> they interact with
> > the rest of the descriptions in the td file? Surprisingly, Google is not
> my friend today
>
> > Specifically why do I get error in pattern definitions once I add
> MIOperandInfo? Ex:
>
> I think both of your errors are because of incompatibility between so_imm,
> so_imm_neg and so_imm_not.
>
> The SUB pattern is trying to use so_imm_neg to match an instruction
> defined with (ins so_imm:$imm) and the MVN similarly has a pattern in the
> instruction using so_imm_not.
>
> Both of the others are still expecting to produce a single immediate,
> rather than two. Hence the problem.
>
> Unfortunately ComplexPatterns are even less well-documented than many
> other parts of TableGen (and have weird and wonderful restrictions on how
> they can be applied). I think the best is
> include/llvm/Target/TargetSelectionDAG.td. Other than that it's the
> TableGen source.
>
> > But here is the problem: since this kind of immediate may be affected by
> fixups, it will be potentially
> > modified outside the encoder/decoder/printer/render methods which are
> tied to the operand class.
> > Thus I actually need to know based on an opcode:
>
> What do you see modifying the operand elsewhere that needs to know those
> details?
>
> I think AArch64's MOVZ/MOVK instructions implement a scheme very similar
> to what I'm describing, and need fixups too. It looks like, in the case
> where a fixup is needed, I made the imm16 part reference the symbol (and
> hence get the fixup) and put a plain 0 imm into the shift amount.
>
> > Also isn't a bitwise representation closer to the "policy" of
> representing operands as close
> > to the encoding as possible?
>
> I don't think so. The fields happen to be listed as "imm12" but it's clear
> as soon as you read the description that it's really two separate fields.
> Especially once the general syntax comes into play.
>
> > Surely representing a floating point as a <sign, mantissa, exp> isn't
> any more accurate.
>
> Now that has merit. Though floats have a history of being a generally
> recognised primitive type, which I don't think applies here.
>
> > Additionally, it does not matter which encoding gets generated for
> selected code.
> > I only need to preserve the exact representation for assembled code...
>
> I think that's equally easy with bitwise or two separate operands.
>
> In the end, though, I'm not going to reject a patch that puts everything
> into a single immediate. If you really think that's neater (and not just
> easier) then go ahead, you're the one doing the work after all.
>
> Cheers.
>
> Tim.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130813/da302570/attachment.html>