[llvm-commits] Speeding up instruction selection

Roman Levenstein romix.llvm at googlemail.com
Wed Apr 16 09:19:41 PDT 2008


Hi Dan,

2008/4/8, Dan Gohman <gohman at apple.com>:
>
>  On Mar 31, 2008, at 5:09 AM, Roman Levenstein wrote:
>
> > Hi Dan, Hi Evan,
>  >
>  > Here is the updated patch.
>  >
>  > 1) I introduced the SDOperandPtr class, as I suggested in my last
>  > email. This class is a sort of an "intelligent" pointer. With this
>  > class, there is no need to copy the SDOperand arrays into the
>  > temporary SmallVectors. Therefore there is no additional overhead.
>
>
> I don't know about *no* additional overhead :-), but I don't
>  expect it'll be a big deal. I didn't expect the previous copying
>  solution to be a big deal either, but I think this new approach
>  is fine.

OK.

>  Please add a doxygen comment for the SDOperandPtr class to
>  document it.

Done.

>  In changes like this in SelectionDAG.h:
>
>  -                    const SDOperand *Ops, unsigned NumOps);
>  +                    const SDOperandPtr Ops, unsigned NumOps);
>
>  the const is now superfluous.
>
>  In the SDOperandPtr class,
>
>  +  SDOperandPtr(SDOperand * op_ptr) {
>  +    ptr = op_ptr;
>  +    object_size = sizeof(SDOperand);
>  +  }
>  +
>  +  SDOperandPtr(const SDOperand * op_ptr) {
>  +    ptr = op_ptr;
>  +    object_size = sizeof(SDOperand);
>  +  }
>
>  The first of these two constructors is unnecessary because
>  the second can be used in both cases.

Done.

>  > 3) I also changed the return types of some methods from SDUse to
>  > SDOperand, as it was suggested during the review.
>
>
> Ok. Remember that Chris also suggested that many of these methods
>  can be changed to return an SDOperand by value instead of by
>  reference. If you decide to do that, please do it with a
>  separate commit.

I haven't done it so far.

>  With the issues mentioned above addressed, this patch looks good
>  to me.

Good. I committed the patch including all improvements suggested by you.

BTW, Dan, have you had any time to look at the patch introducing
queues instead of heaps in the target specific instruction selectors?

-Roman



More information about the llvm-commits mailing list