Use two ComplexPatterns (possible bug of TableGen?)

It seems that it's not allowed to two same 'ComplexPattern's in a 'def',
because TableGen generate the same variable names for the two ComplexPatterns.
If I understand the source code of TableGen correctly, it's not designed to
use more than one ComplexPattern instance (no matter they are the same or not).

In the following example, two 'regsw' are used to match the operands of mul.

def regsw : Operand<v4i32>,
            ComplexPattern<v4i32, 2, "SelectRegsw", > {
  let PrintMethod = "printSrcReg";
  let MIOperandInfo = (ops VR128, i8imm);
}

def MUL_1 : FooInst<(outs VR128:$dst),
                    (ins regsw:$src0, regsw:$src1),
                    "mul $dst, $src0, $src1",
                    [(set VR128:$dst, (mul regsw:$src0, regsw:$src1))]>;

The code generate by TableGen is:

SDNode *Select_ISD_MUL_v4i32(const SDValue &N) {
  SDValue N0 = N.getOperand(0);
  SDValue CPTmp0;
  SDValue CPTmp1;
  if (SelectRegsw(N, N0, CPTmp0, CPTmp1)) {
    SDValue N1 = N.getOperand(1);
    SDValue CPTmp0;
    SDValue CPTmp1;
    if (SelectRegsw(N, N1, CPTmp0, CPTmp1)) {
      return Emit_1(N, FooVS::MUL_1, MVT::v4i32, CPTmp0, CPTmp1);
    }
  }

Alex schrieb:

It seems that it's not allowed to two same 'ComplexPattern's in a 'def',
because TableGen generate the same variable names for the two ComplexPatterns.
If I understand the source code of TableGen correctly, it's not designed to
use more than one ComplexPattern instance (no matter they are the same or not).

I've run into this too, the problem is that variables generated for the
ComplexPattern get the same name(s).

I have a patch against llvm 2.4 that fixes this issue, but did not have
the time to post the patch here. I'll do so by tomorrow.

florian

here is the patch, still against llvm 2.4. I had a short look on trunk, but it
seems that there are several conflicts. Maybe a tablgen expert should have a
look at this - I also do not know if there are changes needed for the
FastISelEmitter.

I hope this is a starting point for you,
florian

ps: We have a working compiler here that relies on these changes, so it does
not seem to break anything, at least for our private backend.

multiple-complexpatterns.patch (2.89 KB)

Hi Florian,

Thanks for the patch! I applied the parts of the patch that rename the
CPTmp variables:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090112/072209.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090112/072210.html

However, I didn't apply this part:

- if (InstPatNode && InstPatNode->getOperator()->getName() == "set") {
+ if (InstPatNode && !InstPatNode->isLeaf() &&
+ InstPatNode->getOperator()->getName() == "set") {

because I'm unsure what it's for. When is a "set" node a leaf?

BTW, the FastISelEmitter should be ignoring any instructions that
have ComplexPatterns, so there shouldn't be a problem there. Also,
it's only enabled for x86 right now.

Dan

Hi Dan,

thank you for applying the patch.

However, I didn't apply this part:

- if (InstPatNode && InstPatNode->getOperator()->getName() ==
"set") {
+ if (InstPatNode && !InstPatNode->isLeaf() &&
+ InstPatNode->getOperator()->getName() == "set") {

because I'm unsure what it's for. When is a "set" node a leaf?

exactly it never is, however InstPatNode may well be a leaf. in that case
calling getOperator is not valid, because it asserts on !isLeaf:

Record *getOperator() const { assert(!isLeaf()); return Operator; }

so this check definitely should be there. but it is true, i should have split
the patch into two pieces as it is not related to the ComplexPattern issue.
sorry for that.

florian

Ah, thanks. I applied this portion of the patch now.

Dan