[llvm-commits] [llvm] r49795 - in /llvm/trunk: include/llvm/CodeGen/ScheduleDAG.h include/llvm/CodeGen/SelectionDAG.h include/llvm/CodeGen/SelectionDAGNodes.h lib/CodeGen/SelectionDAG/DAGCombiner.cpp lib/CodeGen/SelectionDAG/LegalizeDAG.cpp lib/C

[Roman Levenstein]

Hi Dan,

2008/4/16, Dan Gohman <gohman at apple.com>:
> Hi Roman,
>
>  I have a few more comments, not on this commit specifically, but
>  on this overall series.
>
>
>  On Apr 16, 2008, at 9:15 AM, Roman Levenstein wrote:
>  >
>
> > -/// SDOperand - Represents a use of the SDNode referred by
>  > -/// the SDOperandImpl.
>  > -class SDOperand: public SDOperandImpl {
>  > +/// SDUse - Represents a use of the SDNode referred by
>  > +/// the SDOperand.
>  > +class SDUse {
>  > +  SDOperand Operand;
>  >   /// parent - Parent node of this operand.
>
>
> Can you rename parent? I know the public accessor is already
>  renamed, but I'd like the private member's name to match
>  too :-).

Done and committed. Also your const-related comment is taken into account.

>  >
>  >     /// Retrieve a reference to the current operand.
>  > -    SDOperand &operator*() const {
>  > +    SDUse &operator*() const {
>  >       assert(Op && "Cannot dereference end iterator!");
>  >       return *Op;
>  >     }
>  >
>  >     /// Retrieve a pointer to the current operand.
>  > -    SDOperand *operator->() const {
>  > +    SDUse *operator->() const {
>  >       assert(Op && "Cannot dereference end iterator!");
>  >       return Op;
>
>
> (this is in SDNode::use_iterator)
>
>  Now that the SDUse change is in we can take another
>  look at use_iterator::operator* (and operator->).
>  The parallel to Use::value_use_iterator would be to have
>  it return the user SDNode * (i.e., operator* calls
>  getUser).
>
>  At first glance, that seems a little questionable because
>  it doesn't convey which value is being used.

Yes. It does ;-)

> But I think  that's addressed if we persue the idea mentioned earlier
>  about providing a way to iterate through the users of a
>  specific value of a node.
>
>  What do you think?

So,  we should have iterators that iterate only over the users of a
specific value. Do you mean something like providing a constructor
parameter with the specific value number and then having the iterator
pointing only to the uses of this value and skipping everything not
matching this value?

I attach a possible patch implementing this. Please have a look and
tell me if this is what you mean.

-Roman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SDNodeUses.patch6
Type: application/octet-stream
Size: 14743 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20080417/93e71281/attachment.obj>