[PATCH] Improved TableGen support for PowerPC pre-inc operand matching

[Ulrich Weigand]

Hello,

as discussed in previous patches:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130114/162560.html

I'd like to normalize the pre-increment patterns in the PowerPC back-end to
use the same memory operand formats as other instructions with memory
operands, in order to simplify creating an assembler parser for the
platform.  The problem with this is that instruction selection common code
creates "indexed store" nodes that have two separate operands, one for the
base register to be updated, and another for the index register or
displacement.  It is currently not easily possible to match this pair of
two operands on the SelectionDAG level onto a single memory operand at the
instruction level.

The initial attempt in the patch linked to above solved this in the same
way as the ARM back-end currently does, that is, by selecting temporary MI
instructions with two operands, which are later converted to the
single-operand form via custom inserters.  This is admittedly somewhat of a
hack, as was pointed out by Hal as well ...

I've now come up with an alternate approach.  This involves extending the
common TableGen code to allow writing "Pat" patterns that produce an output
instruction having a single operand (with multiple sub-operands) by
providing the sub-operands as distinct operands at the pattern level.  This
way, we can map the SelectionDAG pre_store etc. nodes onto instructions by
straight-forward patterns along the line of:

def : Pat<(pre_store GPRC:$rS, ptr_rc:$ptrreg, iaddroff:$ptroff),
            (STWU GPRC:$rS, iaddroff:$ptroff, ptr_rc:$ptrreg)>;

where STWU is defined with a single operand of type "memri" (with multiple
sub-operands):

def STWU  : DForm_1<37, (outs ptr_rc:$ea_res), (ins GPRC:$rS, memri:$dst),
                    "stwu $rS, $dst", LdStStoreUpd, []>,
                    RegConstraint<"$dst.reg = $ea_res">,
NoEncode<"$ea_res">;

I've found two places in TableGen that required changes in order to support
this usage: TreePatternNode::ApplyTypeConstraints, where operand types are
propagated and checked for compatibility, and
MatcherGen::EmitResultInstructionAsOperand, where code to emit resulting
operands is actually generated.  In both places, current code makes the
implicit assumption that an instruction operand with multiple sub-operand
must be matched by a ComplexPattern operand with the same number of
sub-operands.  With this patch, this restriction is relaxed to also allow a
series of simple operands instead.

Note that while the code currently *assumes* the match-up just described,
it doesn't actually *verify* this.  This leads to cases like the X86_64
pattern described here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130204/164415.html
which seems to be simply incorrect.  As the new code performs stricter
checks, this pattern now causes a compile-time error, and thus needs to be
fixed one way or the other (possibly by just removing it?) before the patch
can be applied.

[ Another problem detected by the stricter checks was a bit-size mismatch
between the "tocentry" operand and its sole sub-operand in the PowerPC
back-end.   This is fixed in the patch as well. ]


Apart from the X86 hack, and obviously changes to PowerPC, the TableGen
patch does *not* result in any changes to generated TableGen files on any
platform.  The patch also passes bootstrap and regression testing on
powerpc64-linux.


Any comments welcome!   Assuming a solution for the X86 TLS hack is found,
would this patch be acceptable to commit?

Thanks,
Ulrich

(See attached file: diff-llvm-asm-preinc)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff-llvm-asm-preinc
Type: application/octet-stream
Size: 31623 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130205/234bd1ad/attachment.obj>