[llvm-commits] [llvm] r47607 - in /llvm/trunk: include/llvm/Instructions.h lib/VMCore/Instructions.cpp

Chris Lattner clattner at apple.com
Tue Feb 26 10:26:24 PST 2008


> URL: http://llvm.org/viewvc/llvm-project?rev=47607&view=rev
> Log:
> Optimize most common case by using single RetVal in ReturnInst.

Nice!

>
> @@ -1405,6 +1406,23 @@
>
>   virtual ReturnInst *clone() const;
>
> +  // Transparently provide more efficient getOperand methods.
> +  Value *getOperand(unsigned i) const {
> +    assert(i < getNumOperands() && "getOperand() out of range!");
> +    if (getNumOperands() == 0 || getNumOperands() == 1)
> +      return RetVal;
> +
> +    return OperandList[i];
> +  }
> +
> +  void setOperand(unsigned i, Value *Val) {
> +    assert(i < getNumOperands() && "setOperand() out of range!");
> +    if (i == 0)
> +      RetVal = Val;
> +    else
> +      OperandList[i] = Val;
> +  }

I would suggest just dropping these two methods: it's likely that  
dereferencing OperandList unconditionally is faster or as fast than  
conditional code.


> ReturnInst::ReturnInst(const ReturnInst &RI)
>   : TerminatorInst(Type::VoidTy, Instruction::Ret,
> -                   OperandList, RI.getNumOperands()) {
> +                   &RetVal, RI.getNumOperands()) {
>   unsigned N = RI.getNumOperands();
> +  if (N == 1)
> +    RetVal.init(RI.RetVal, this);
> +  else if (N) {
> +    Use *OL = OperandList = new Use[N];
> +    for (unsigned i = 0; i < N; ++i)
> +      OL[i].init(RI.getOperand(i), this);
> +  }

Nice, please set OperandList to null in the N = 0 case though.  The  
best way to do this is to pass up null instead of &RetVal as the base.

> Value *ReturnInst::getReturnValue(unsigned n) const {
> +  if (getNumOperands() == 0)
>     return 0;

Is this needed?  calling getReturnValue(0) should assert.

> +  assert (n < getNumOperands() && "getReturnValue out of range!");
> +  if (getNumOperands() == 1)
> +    return RetVal;
> +  else
> +    return OperandList[n];
> }

I think this whole method should just be an inline method that just  
does: return getOperand(n);

no?

-Chris




More information about the llvm-commits mailing list