[llvm-commits] [llvm] r55085 - in /llvm/trunk: include/llvm/CodeGen/FastISel.h lib/CodeGen/SelectionDAG/FastISel.cpp lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

[Dan Gohman]

Hi Evan,

Comments inline below.

On Aug 20, 2008, at 3:45 PM, Evan Cheng wrote:

> Author: evancheng
> Date: Wed Aug 20 17:45:34 2008
> New Revision: 55085
>
> URL: http://llvm.org/viewvc/llvm-project?rev=55085&view=rev
> Log:
> First cut, un-optimized (and untested) fast isel lowering of  
> GetElementPtrInst.
>
> Modified:
>    llvm/trunk/include/llvm/CodeGen/FastISel.h
>    llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/FastISel.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/FastISel.h?rev=55085&r1=55084&r2=55085&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/include/llvm/CodeGen/FastISel.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/FastISel.h Wed Aug 20 17:45:34  
> 2008
> @@ -23,7 +23,9 @@
> class MachineBasicBlock;
> class MachineFunction;
> class MachineRegisterInfo;
> +class TargetData;
> class TargetInstrInfo;
> +class TargetLowering;
> class TargetRegisterClass;
>
> /// FastISel - This is a fast-path instruction selection class that
> @@ -33,7 +35,9 @@
>   MachineBasicBlock *MBB;
>   MachineFunction &MF;
>   MachineRegisterInfo &MRI;
> +  const TargetData &TD;
>   const TargetInstrInfo &TII;
> +  TargetLowering &TLI;
>
> public:
>   /// SelectInstructions - Do "fast" instruction selection over the
> @@ -72,6 +76,29 @@
>                                ISD::NodeType Opcode,
>                                unsigned Op0, unsigned Op1);
>
> +  /// FastEmit_i - This method is called by target-independent code
> +  /// to request that an instruction with the given type which  
> materialize
> +  /// the specified immediate value.
> +  virtual unsigned FastEmit_i(MVT::SimpleValueType VT, uint64_t Imm);
> +
> +  /// FastEmit_ri - This method is called by target-independent code
> +  /// to request that an instruction with the given type, opcode, and
> +  /// register and immediate operands be emitted.
> +  ///
> +  virtual unsigned FastEmit_ri(MVT::SimpleValueType VT,
> +                               ISD::NodeType Opcode,
> +                               unsigned Op0, uint64_t Imm,
> +                               MVT::SimpleValueType ImmType);
> +
> +  /// FastEmit_ri_ - This method is a wrapper of FastEmit_ri. It  
> first tries
> +  /// to emit an instruction with an immediate operand using  
> FastEmit_ri.
> +  /// If that fails, it materializes the immediate into a register  
> and try
> +  /// FastEmit_rr instead.
> +  unsigned FastEmit_ri_(MVT::SimpleValueType VT,
> +                        ISD::NodeType Opcode,
> +                        unsigned Op0, uint64_t Imm,
> +                        MVT::SimpleValueType ImmType);
> +
>   /// FastEmitInst_ - Emit a MachineInstr with no operands and a
>   /// result register in the given register class.
>   ///
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp?rev=55085&r1=55084&r2=55085&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp Wed Aug 20  
> 17:45:34 2008
> @@ -15,7 +15,9 @@
> #include "llvm/CodeGen/FastISel.h"
> #include "llvm/CodeGen/MachineInstrBuilder.h"
> #include "llvm/CodeGen/MachineRegisterInfo.h"
> +#include "llvm/Target/TargetData.h"
> #include "llvm/Target/TargetInstrInfo.h"
> +#include "llvm/Target/TargetLowering.h"
> #include "llvm/Target/TargetMachine.h"
> using namespace llvm;
>
> @@ -48,8 +50,81 @@
>
> bool FastISel::SelectGetElementPtr(Instruction *I,
>                                    DenseMap<const Value*, unsigned>  
> &ValueMap) {
> -  // TODO: implement me
> -  return false;
> +  unsigned N = ValueMap[I->getOperand(0)];
> +  if (N == 0)
> +    // Unhandled operand. Halt "fast" selection and bail.
> +    return false;
> +
> +  const Type *Ty = I->getOperand(0)->getType();
> +  MVT VT = MVT::getMVT(Ty, /*HandleUnknown=*/true);

HandleUnknown should be false here; the type of a GEP is a pointer,
which shouldn't ever be unknown.

>
> +  MVT::SimpleValueType PtrVT = TLI.getPointerTy().getSimpleVT();

PtrVT and VT will have the same value here. Can one of them be  
eliminated?

>
> +
> +  for (GetElementPtrInst::op_iterator OI = I->op_begin()+1, E = I- 
> >op_end();
> +       OI != E; ++OI) {
> +    Value *Idx = *OI;
> +    if (const StructType *StTy = dyn_cast<StructType>(Ty)) {
> +      unsigned Field = cast<ConstantInt>(Idx)->getZExtValue();
> +      if (Field) {
> +        // N = N + Offset
> +        uint64_t Offs = TD.getStructLayout(StTy)- 
> >getElementOffset(Field);
> +        // FIXME: This can be optimized by combining the add with a
> +        // subsequent one.
> +        N = FastEmit_ri(VT.getSimpleVT(), ISD::ADD, N, Offs, PtrVT);
> +        if (N == 0)
> +          // Unhandled operand. Halt "fast" selection and bail.
> +          return false;
> +      }
> +      Ty = StTy->getElementType(Field);
> +    } else {
> +      Ty = cast<SequentialType>(Ty)->getElementType();
> +
> +      // If this is a constant subscript, handle it quickly.
> +      if (ConstantInt *CI = dyn_cast<ConstantInt>(Idx)) {
> +        if (CI->getZExtValue() == 0) continue;
> +        uint64_t Offs =
> +          TD.getABITypeSize(Ty)*cast<ConstantInt>(CI)- 
> >getSExtValue();
> +        N = FastEmit_ri(VT.getSimpleVT(), ISD::ADD, N, Offs, PtrVT);
> +        if (N == 0)
> +          // Unhandled operand. Halt "fast" selection and bail.
> +          return false;
> +        continue;
> +      }
> +
> +      // N = N + Idx * ElementSize;
> +      uint64_t ElementSize = TD.getABITypeSize(Ty);
> +      unsigned IdxN = ValueMap[Idx];
> +      if (IdxN == 0)
> +        // Unhandled operand. Halt "fast" selection and bail.
> +        return false;
> +
> +      // If the index is smaller or larger than intptr_t, truncate  
> or extend
> +      // it.
> +      MVT IdxVT = MVT::getMVT(Idx->getType(), /*HandleUnknown=*/ 
> true);

Same as above with HandleUnknown.

>
> +      if (IdxVT.bitsLT(VT))
> +        IdxN = FastEmit_r(VT.getSimpleVT(), ISD::SIGN_EXTEND, IdxN);
> +      else if (IdxVT.bitsGT(VT))
> +        IdxN = FastEmit_r(VT.getSimpleVT(), ISD::TRUNCATE, IdxN);
> +      if (IdxN == 0)
> +        // Unhandled operand. Halt "fast" selection and bail.
> +        return false;
> +
> +      // FIXME: If multiple is power of two, turn it into a shift.  
> The
> +      // optimization should be in FastEmit_ri?
> +      IdxN = FastEmit_ri(VT.getSimpleVT(), ISD::MUL, IdxN,
> +                         ElementSize, PtrVT);

How would converting a multiply into a shift benefit compile time?

Dan