On Wed, Apr 20, 2011 at 10:58 AM, Justin Holewinski <[email protected]> wrote: > > > On Wed, Apr 20, 2011 at 1:52 PM, Eli Friedman <[email protected]> > wrote: >> >> On Wed, Apr 20, 2011 at 10:50 AM, Eli Friedman <[email protected]> >> wrote: >> > On Wed, Apr 20, 2011 at 10:14 AM, Justin Holewinski >> > <[email protected]> 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. >> >> 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. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
