[cfe-commits] [PATCH] Add ptx32 and ptx64 target triples, as well as PTX builtins

Justin Holewinski justin.holewinski at gmail.com
Wed Apr 20 12:15:29 PDT 2011


On Wed, Apr 20, 2011 at 2:17 PM, Eli Friedman <eli.friedman at gmail.com>wrote:

> On Wed, Apr 20, 2011 at 10:58 AM, Justin Holewinski
> <justin.holewinski at gmail.com> wrote:
> >
> >
> > On Wed, Apr 20, 2011 at 1:52 PM, Eli Friedman <eli.friedman at gmail.com>
> > wrote:
> >>
> >> On Wed, Apr 20, 2011 at 10:50 AM, Eli Friedman <eli.friedman at gmail.com>
> >> wrote:
> >> > On Wed, Apr 20, 2011 at 10:14 AM, Justin Holewinski
> >> > <justin.holewinski at gmail.com> wrote:
> >> >> The attached patch adds target triples for the PTX back-end, and adds
> >> >> the
> >> >> currently implemented PTX intrinsics as builtin functions.  I would
> >> >> like for
> >> >> someone familiar with the Clang targets/triples and builtins code to
> >> >> review
> >> >> this patch.  I have been involved in the PTX back-end for several
> >> >> months
> >> >> now, but this is my first patch to Clang.
> >> >
> >> > +    PTXTargetInfo(const std::string& triple) : TargetInfo(triple) {
> >> > +      TLSSupported = false;
> >> > +      IntWidth = IntAlign = 32;
> >> > +      LongWidth = LongLongWidth = LongAlign = LongLongAlign = 64;
> >> > +    }
> >> >
> >> > You can skip setting IntWidth/IntAlign/LongLongWidth/LongLongAlign;
> >> > the defaults are correct.
> >
> > Okay, will do.
> >
> >>
> >> >
> >> > +  class PTX32TargetInfo : public PTXTargetInfo {
> >> > +  public:
> >> > +  PTX32TargetInfo(const std::string& triple) : PTXTargetInfo(triple)
> {
> >> > +      PointerWidth = PointerAlign = 32;
> >> > +      DescriptionString
> >> > +        =
> "e-p:32:32-i64:32:32-f64:32:32-v128:32:128-v64:32:64-n32:64";
> >> > +    }
> >> > +  };
> >> >
> >> > The target description string isn't using the same alignment for long
> >> > and for double as the clang TargetInfo.  Also, the alignment for v64
> >> > and v128 looks wrong.
> >
> > Would there be any issue with just removing the v64 and v128 specifiers
> for
> > now?  The back-end does not currently support them.
>
> That's fine.
>
> >>
> >> >
> >> > BUILTIN(__builtin_ptx_read_tid_x, "i", "nc")
> >> >
> >> > The "c" means that the value returned by these builtins never changes;
> >> > that doesn't seem right.
> >> >
> >
> > This is an interesting case.  I assumed the "const" meant that successive
> > calls to the builtin will return the same value (which is true).
>  However,
> > different threads will see different values.  What is the recommended
> > behavior here?  For a given piece of C code that will compile down to
> PTX,
> > it is perfectly valid for constant propagation to eliminate any
> successive
> > calls.
>
> It should be fine for __builtin_ptx_read_tid_x, but it seems a bit
> weird for  __builtin_ptx_read_clock, and very wrong for
> __builtin_ptx_bar_sync.
>

Oops, that is a copy-paste error.


>
> >>
> >> Oh, one more thing: this description unconditionally defines size_t
> >> and friends to a 64-bit type; that isn't wrong, exactly, but probably
> >> not what you want.
> >
> > Where exactly is this defined?  I'm not too familiar with Clang's
> TargetInfo
> > class yet.
>
> SizeType, PtrDiffType, IntPtrType.
>

Fixed.


>
> -Eli
>

Attached is an updated patch.

-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110420/b956b064/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-PTX-Add-PTX-intrinsics-as-builtins-and-add-ptx32-and.patch
Type: application/octet-stream
Size: 9499 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110420/b956b064/attachment.obj>


More information about the cfe-commits mailing list