[llvm-commits] Speeding up instruction selection

Roman Levenstein romix.llvm at googlemail.com
Fri Mar 28 14:58:28 PDT 2008


Hi,

Sorry for the previous non-finished email. I clicked the wrong button :-)

2008/3/29, Dan Gohman <gohman at apple.com>:
>  >> I also have a comment on the overall set of changes. Right now
>  >> SDUse inherits from SDOperand, and that makes SDUse* implicitly
>  >> convertible to SDOperand*, which as you noticed is unsafe when
>  >> there's pointer arithmetic.
>  >
>  > I also thought about this issue... To experiement, I tried to make
>  > SDOperand a provate base of SDUse, to see where it would break.
>  > Basically, you cannot use the SmallVector approach afterwards.
>  >
>  >> I think it'd be better for SDUse to have an SDOperand member
>  >> instead, with associated
>  >> accessor methods.
>  > I had this idea as well.
>  >
>  >
>  >> To keep op_iterator working, it may be easiest to add
>  >> an implicit conversion from SDUse to SDOperand; this happens
>  >> be analogous to what llvm::Use does. On the other hand, an
>  >> op_iterator class wouldn't be out of place here either. What
>  >> do you think?
>  >
>  > I'm not sure this would really help much. Many places expect pointers
>  > to the array of SDOperands and unfortunately, many of them are used
>  > for real SDOperand array and for OperandList-kind of operands at the
>  > same time. Conversion of the single pointer does not help here. So, we
>  > either need SmallVector-trick at some places or we should continue
>  > using SDOperand instead of SDUse. That is, SDUse becomes the SDOperand
>  > again, as I did before.
>
>
> I'm saying we should keep the SmallVector trick (for now), and
>  that we can make it work without inheritance. Adding an implicit
>  conversion to SDOperand in SDUse:
>
>    operator SDOperand() const { return MySDOperand; }
>
>  is enough to let the SmallVector trick work. This has the
>  nice property of being the same solution that the high-level
>  Use uses.

OK. I see. This would work. But this does not solve the potential
performance problem that Evan mentioned. I have a better idea for a
solution:
Everywhere, where we pass SDOperand *, we should pass a special
iterator (or some sort of smart pointer) called e.g.  SDOperandPtr.
Internally such a pointer has a structure like this:
struct SDOperandPtr {
   SDOperand *ptr;
   int object size;
   SDOperand operator *() { return *ptr;}
   operator ++ () { ptr = (SDOperand*)((char *)ptr + size);}
   SDOperandPtr(SDUse * use_ptr) { ptr = use_ptr; size = sizeof(SDUse);}
   SDOperandPtr(SDOperand * op_ptr) {ptr = op_ptr; size = sizeof(SDOperand);}
};
This is just a brain dump, not tested yet. But in theory, this permits
us to work with such a pointer like with the usual SDOperand pointer,
except that it can correctly iterate over arrays of SDUse objects as
well. And no copying is required compared with the SmallVector
approach.

What do you think?

- Roman



More information about the llvm-commits mailing list