[llvm-commits] [llvm] r47430 - in /llvm/trunk: docs/LangRef.html include/llvm/CodeGen/SelectionDAG.h include/llvm/CodeGen/SelectionDAGNodes.h include/llvm/Intrinsics.td include/llvm/Target/TargetLowering.h lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

[Andrew Lenharth]

On 2/21/08, Evan Cheng <evan.cheng at apple.com> wrote:
>
>  On Feb 20, 2008, at 10:45 PM, Andrew Lenharth wrote:
>
>  Hi,
>
>  Some comments below.
> > ======================================================================
>  > --- llvm/trunk/include/llvm/CodeGen/SelectionDAG.h (original)
>  > +++ llvm/trunk/include/llvm/CodeGen/SelectionDAG.h Thu Feb 21
>  > 00:45:13 2008
>  > @@ -356,6 +356,16 @@
>  >   SDOperand getVAArg(MVT::ValueType VT, SDOperand Chain, SDOperand
>  > Ptr,
>  >                      SDOperand SV);
>  >
>  > +  /// getAtomic - Gets a node for an atomic op, produces result and
>  > chain, takes
>  > +  // 3 operands
>
>  // -> ///
>
>  >
>  > +  SDOperand getAtomic(unsigned Opcode, SDOperand Chain, SDOperand
>  > Ptr,
>  > +                      SDOperand A2, SDOperand A3, MVT::ValueType VT);
>
>
> I know you described the operands in the doc. But please choose better
>  names than A2 and A3.

OK.

>
>  >
>  > +
>  > +  /// getAtomic - Gets a node for an atomic op, produces result and
>  > chain, takes
>  > +  // 2 operands
>  > +  SDOperand getAtomic(unsigned Opcode, SDOperand Chain, SDOperand
>  > Ptr,
>  > +                      SDOperand A2, MVT::ValueType VT);
>  > +
>  >   /// getLoad - Loads are not normal binary operators: their result
>  > type is not
>  >   /// determined by their operands, and they produce a value AND a
>  > token chain.
>  >   ///
>  >
>  > Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h
>  > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h?rev=47430&r1=47429&r2=47430&view=diff
>  >
>  > =
>  > =
>  > =
>  > =
>  > =
>  > =
>  > =
>  > =
>  > ======================================================================
>  > --- llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h (original)
>  > +++ llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h Thu Feb 21
>  > 00:45:13 2008
>  > @@ -441,7 +441,7 @@
>  >     // is added / subtracted from the base pointer to form the
>  > address (for
>  >     // indexed memory ops).
>  >     LOAD, STORE,
>  > -
>  > +
>  >     // DYNAMIC_STACKALLOC - Allocate some number of bytes on the
>  > stack aligned
>  >     // to a specified boundary.  This node always has two return
>  > values: a new
>  >     // stack pointer value and a chain. The first operand is the
>  > token chain,
>  > @@ -591,12 +591,30 @@
>  >
>  >     // OUTCHAIN = MEMBARRIER(INCHAIN, load-load, load-store, store-
>  > load,
>  >     //                       store-store, device)
>  > -    // This corresponds to the atomic.barrier intrinsic.
>  > +    // This corresponds to the memory.barrier intrinsic.
>  >     // it takes an input chain, 4 operands to specify the type of
>  > barrier, an
>  >     // operand specifying if the barrier applies to device and
>  > uncached memory
>  >     // and produces an output chain.
>  >     MEMBARRIER,
>  >
>  > +    // Val, OUTCHAIN = ATOMIC_LCS(INCHAIN, ptr, cmp, swap)
>  > +    // this corresponds to the atomic.lcs intrinsic.
>  > +    // cmp is compared to *ptr, and if equal, swap is stored in *ptr.
>  > +    // the return is always the original value in *ptr
>  > +    ATOMIC_LCS,
>  > +
>  > +    // Val, OUTCHAIN = ATOMIC_LAS(INCHAIN, ptr, amt)
>  > +    // this corresponds to the atomic.las intrinsic.
>  > +    // *ptr + amt is stored to *ptr atomically.
>  > +    // the return is always the original value in *ptr
>  > +    ATOMIC_LAS,
>  > +
>  > +    // Val, OUTCHAIN = ATOMIC_SWAP(INCHAIN, ptr, amt)
>  > +    // this corresponds to the atomic.swap intrinsic.
>  > +    // amt is stored to *ptr atomically.
>  > +    // the return is always the original value in *ptr
>  > +    ATOMIC_SWAP,
>  > +
>  >     // BUILTIN_OP_END - This must be the last enum value in this list.
>  >     BUILTIN_OP_END
>  >   };
>  > @@ -1170,6 +1188,33 @@
>  >   SDOperand getValue() const { return Op; }
>  > };
>  >
>  > +class AtomicSDNode : public SDNode {
>  > +  virtual void ANCHOR();  // Out-of-line virtual method to give
>  > class a home.
>  > +  SDOperand Ops[4];
>  > +  MVT::ValueType OrigVT;
>  > +public:
>  > +  AtomicSDNode(unsigned Opc, SDVTList VTL, SDOperand Chain,
>  > SDOperand X,
>  > +               SDOperand Y, SDOperand Z, MVT::ValueType VT)
>  > +    : SDNode(Opc, VTL) {
>  > +    Ops[0] = Chain;
>  > +    Ops[1] = X;
>  > +    Ops[2] = Y;
>  > +    Ops[3] = Z;
>
>
> Do you need an assertion  X.getValueType() == Y.getValueType()?

yea.

>
>  >
>  > +    InitOperands(Ops, 4);
>  > +    OrigVT=VT;
>  > +  }
>  > +  AtomicSDNode(unsigned Opc, SDVTList VTL, SDOperand Chain,
>  > SDOperand X,
>  > +               SDOperand Y, MVT::ValueType VT)
>  > +    : SDNode(Opc, VTL) {
>  > +    Ops[0] = Chain;
>  > +    Ops[1] = X;
>  > +    Ops[2] = Y;
>  > +    InitOperands(Ops, 3);
>  > +    OrigVT=VT;
>  > +  }
>  > +  MVT::ValueType getVT() const { return OrigVT; }
>  > +};
>
>
> How about accessors, e.g. getCompare() since you have gone through the
>  trouble of creating a new SDOperand subclass for it.

Good idea.

>  > =
>  > ======================================================================
>  > --- llvm/trunk/include/llvm/Target/TargetLowering.h (original)
>  > +++ llvm/trunk/include/llvm/Target/TargetLowering.h Thu Feb 21
>  > 00:45:13 2008
>  > @@ -1252,7 +1252,7 @@
>  >   /// TargetDAGCombineArray - Targets can specify ISD nodes that
>  > they would
>  >   /// like PerformDAGCombine callbacks for by calling
>  > setTargetDAGCombine(),
>  >   /// which sets a bit in this array.
>  > -  unsigned char TargetDAGCombineArray[156/(sizeof(unsigned char)*8)];
>  > +  unsigned char TargetDAGCombineArray[160/(sizeof(unsigned char)*8)];
>
>
> I don't think this ought be changed.  It should be the same size as
>  OpActions. Although I don't know where 156 comes from. Anyone?

It was overflowing when I added intrinsics, so probably OpActions
should be increased too.


>  > +// Atomic support
>  > +//
>  > +let Defs = [EAX] in
>  > +def LCMPXCHGL : I<0, Pseudo, (outs GR32:$dst),
>  > +                  (ins GR32:$ptr, GR32:$cmp, GR32:$swap),
>  > +                  "movl $cmp, %eax ; lock cmpxchgl $swap,($ptr) ;
>  > movl %eax, $dst",
>  > +                  [(set GR32:$dst, (atomic_lcs_32 GR32:$ptr,
>  > GR32:$cmp, GR32:$swap))]>;
>
>
> How about JIT support? This has to be handled specially in
>  X86CodeEmitter.

I didn't mean this to be the x86 implementation.  cmpxchgl and the
lock prefix will have to be added to the .td file and appropriate
patterns made for all the atomics.  That was just something to have
enough to test with.

Andrew