InstructionSelectBasicBlock question

Hi,

I have two questions about the above method of the SelectionDAGISel class.

1. The overrides in PPCDAGToDAGISel and in I64DAGToDAGISel are identical
(except for PPCISD::FIRST_NUMBER vs. IA64ISD::FIRST_NUMBER).
Maybe, this means that it would be better if SelectionDAGISel had default
implementation? That would remove this code duplication.

If desired, SelectionDAGISel can have both implementation and be pure, so
that derived class authors are forced to think how they want this method to
behave, and still have the option to call the default implementation.

2. What are possible reasons for *not* using the default implementation? X86
has completely different code, why?

Thanks,
Volodya

Vladimir Prus wrote:

I have two questions about the above method of the SelectionDAGISel class.

1. The overrides in PPCDAGToDAGISel and in I64DAGToDAGISel are identical
(except for PPCISD::FIRST_NUMBER vs. IA64ISD::FIRST_NUMBER).
Maybe, this means that it would be better if SelectionDAGISel had default
implementation? That would remove this code duplication.

And the Sparc backend has a much simpler implementation, so it seems that
the huge loop in PPC and I64 implementation is just no longer needed?

- Volodya

I have two questions about the above method of the SelectionDAGISel class.

1. The overrides in PPCDAGToDAGISel and in I64DAGToDAGISel are identical
(except for PPCISD::FIRST_NUMBER vs. IA64ISD::FIRST_NUMBER).
Maybe, this means that it would be better if SelectionDAGISel had default
implementation? That would remove this code duplication.

Yup, that would be great.

2. What are possible reasons for *not* using the default implementation? X86
has completely different code, why?

There are three different cases here:

1. In Sparc, we see simple and logical code to select a block.
2. In X86, it needs to do extra work to deal with the FP stack stuff
    after doing #1. This work is target-specific and shouldn't be needed
    by any other target.
3. In the case of PPC/Itanium, the code is *functionally* identical to #1,
    but includes some hacks that are designed to reduce the stack space
    usage of the instruction selector (darwin/ppc defaults to a really tiny
    stack, so these hacks are worthwhile. The IA64 presumably copied the
    implementation from the PPC backend).

To me, I think it would make sense to move the implementation in the PPC backend up to the SelectionDAGISel class. The stack space reducing hacks can benefit all targets, and the algorithm is the same for all (like you said, except for the enum). The X86 backend can override it, explicitly call the superclass, then do the magic it wants.

Would you be interested in working on this?

-Chris

Chris Lattner wrote:

I have two questions about the above method of the SelectionDAGISel
class.

1. The overrides in PPCDAGToDAGISel and in I64DAGToDAGISel are identical
(except for PPCISD::FIRST_NUMBER vs. IA64ISD::FIRST_NUMBER).
Maybe, this means that it would be better if SelectionDAGISel had default
implementation? That would remove this code duplication.

Yup, that would be great.

2. What are possible reasons for *not* using the default implementation?
X86 has completely different code, why?

There are three different cases here:

1. In Sparc, we see simple and logical code to select a block.
2. In X86, it needs to do extra work to deal with the FP stack stuff
    after doing #1. This work is target-specific and shouldn't be needed
    by any other target.
3. In the case of PPC/Itanium, the code is *functionally* identical to #1,
    but includes some hacks that are designed to reduce the stack space
    usage of the instruction selector (darwin/ppc defaults to a really
    tiny
    stack, so these hacks are worthwhile. The IA64 presumably copied the
    implementation from the PPC backend).

Let me make sure I understand: the hacks are to reduce stack space of the
*instruction selector*? So, if I use PPC codegen on darwin/ppc, things will
work, but if I use X86 codegen on the same darwin/ppc machine, X86 won't
use those stack-space reducing hacks and stack with blow up?

To me, I think it would make sense to move the implementation in the PPC
backend up to the SelectionDAGISel class. The stack space reducing hacks
can benefit all targets, and the algorithm is the same for all (like you
said, except for the enum). The X86 backend can override it, explicitly
call the superclass, then do the magic it wants.

To clarify, the base method should have implementation and should not be
pure, right?

Would you be interested in working on this?

Yes, I hope to do this in near future.

Thanks,
Volodya

    stack, so these hacks are worthwhile. The IA64 presumably copied the
    implementation from the PPC backend).

Let me make sure I understand: the hacks are to reduce stack space of the
*instruction selector*? So, if I use PPC codegen on darwin/ppc, things will
work, but if I use X86 codegen on the same darwin/ppc machine, X86 won't
use those stack-space reducing hacks and stack with blow up?

Yes, exactly.

To me, I think it would make sense to move the implementation in the PPC
backend up to the SelectionDAGISel class. The stack space reducing hacks
can benefit all targets, and the algorithm is the same for all (like you
said, except for the enum). The X86 backend can override it, explicitly
call the superclass, then do the magic it wants.

To clarify, the base method should have implementation and should not be
pure, right?

Correct.

Would you be interested in working on this?

Yes, I hope to do this in near future.

Great!

-Chris