[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/CodeGen/SelectionDAG/LegalizeTypes.cpp lib/CodeGen/SelectionDAG/LegalizeTypes.h lib/CodeGen/SelectionDAG/ScheduleDAG.cpp lib/CodeGen/SelectionDAG/SelectionDAG.cpp lib/Target/X86/X86ISelLowering.cpp

[Dan Gohman]

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 :-).

>
>     /// 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. 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?

Dan