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. > > + 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. > > 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. >
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. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
