Jan Vesely <[email protected]> writes: > On Thu, 2014-04-24 at 12:05 +0200, Francisco Jerez wrote: > <snip> >> >>> >> >>> I don't think that using getTypeAllocSize() instead of >> >>> getTypeStoreSize() to calculate clover::argument::size would be a >> >>> satisfactory solution. By doing that you'd redefine the API argument >> >>> size exposed to the host for *all* argument types to be the >> >>> device-dependent aligned size, which is definitely not what you want. >> >> >> >>> AFAIK 3-element vectors are an exception because they are the only types >> >>> that are defined to have a different API size from their actual usable >> >>> size, so they probably deserve special handling in invocation.cpp (as >> >>> you did in your first patch). As the API size is target-independent I >> >>> don't think that the fix belongs in Clang or LLVM, Clover is likely at >> >>> fault. >> >> >> >> The way I understood the ch 6.1.5 is that both API and OpenCL C 3 >> >> element vectors are required to be 4*sizeof(component). So a >> >> sizeof(float3) == sizeof(cl_float3) == 16, and should be both host and >> >> target independent. >> > >> > Well, sizeof() is supposed to take into account the alignment, so this >> > should be the case already. > > AFAIK sizeof only includes padding. I explain below. >
Yes, and 3-component vectors are being padded to 4 components already. >> > >> >> That's why clang (or more precisely libclc) looked like a correct >> >> place. >> >> >> > >> > Right. I guess the other possibility would be to redefine cl_float3 as >> > cl_float4 in libclc. >> >> Sorry, I meant to redefine "gentype3" as "gentype4" for every vector >> element type "gentype". >> >> > You mentioned that it had undesired consequences. Which exactly? > > With this change: > -typedef __attribute__((ext_vector_type(3))) float float3; > +typedef __attribute__((ext_vector_type(4))) float float3; > > errors about duplicate ops (since type3 and type4 are now the same type) > can be fixed. However, I don't know a clean way to fix the following: > > error: too few elements in vector initialization (expected 4 elements, > have 3) > > Even if we can get rid of it, or reduce it to warning. > having the same type for type3 and type4 causes problem that we won't be > able to distinguish following situations: > > flaot4 A > > float3 B = A.xyz; // This is OK, should not produce warning/error; > > float4 C = A.xyz; // This should at least produce a warning. even if we > allow using fewer elements for vector initialization. > Right, I agree that this wouldn't be a good idea. > >> > >> >> I understand that target device can have stricter alignment rules, but I >> >> don't see how it can have different type sizes (my reading of the specs >> >> is that these are binding for the target as well). >> >> >> > >> > In theory the sizes are binding for most built-in types, but R600 >> > doesn't support certain integer types so clover has code to take into >> > account the differing size, alignment and endianness between host and >> > device. I guess that in most cases it would be possible to use the >> > "official" ABI for passing kernel arguments to the device (and that's a >> > requirement for your solution using DataLayout::getTypeAllocSize() to >> > work reliably, otherwise you'll be taking into account device-specific >> > padding), but you'll have to fix the R600 back-end so it's able to deal >> > with (i.e. lower) all built-in types specified by OpenCL. I think that >> > this would be useful on its own, Tom should know better than I how >> > difficult it would be. > > I didn't know about the additional R600 type size magic, in this case I > agree that the original approach (detect 3elem vectors and change size) > is probably the best in this situation. > >> > >> >> I can resend the original patch with debug output replaced by a comment. >> >> >> > A slightly more general solution than what you did could be to align the >> > store size of scalar and vector arguments to the next power of two to >> > calculate the API size (in particular, that would make 3- and 4-element >> > vectors have the same size). From 6.1.5: "A built-in data type that is >> > not a power of two bytes in size must be aligned to the next larger >> > power of two." > > This part is what made me think that the _alignment_ only restricts > starting address (and not size). It also restricts the size. From section 6.3 on the sizeof operator: "The sizeof operator yields the size (in bytes) of its operand, including any padding bytes (refer to section 6.1.5) needed for alignment [...]. When applied to an operand that has structure or union type, the result is the total number of bytes in such an object, including internal and trailing padding." > Otherwise, the addition of 3 element vector special case would not be > necessary. Maybe it wouldn't be strictly necessary, but it's still clarifying as an illustrative corollary of the general rule. > i.e I think that the > following mem layout is legal in OCL 1.0 but not in OCL1.1+ > > 0x10: float3 here > 0x1c: int here // this should belong to float3 in ocl 1.1+ Does OpenCL 1.0 support 3-component vectors at all? > > while the following is illegal in both: > > 0x8: float3 here > > So, I don't think that the quoted text requires all builtin types to > have 2^x size (although all but 3 element vectors do). > Well, don't interpret it that way if you don't want to, but all OpenCL built-in types do (or at least the ones you are allowed to use as kernel argument types), so this solution seems more elegant to me than special-casing 3-component vectors. Thanks. > regards, > Jan > > >> > >> >> regards, >> >> Jan >> >> >> > >> > Thanks. >> > >> >>> >> >>> Thanks. >> >>> >> >>> > regards, >> >>> > Jan >> >>> > >> >>> > >> >>> > -- >> >>> > Jan Vesely <[email protected]> >> >>> > _______________________________________________ >> >>> > mesa-dev mailing list >> >>> > [email protected] >> >>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> >> >> >> -- >> >> Jan Vesely <[email protected]>
pgpPdnoYxF6rV.pgp
Description: PGP signature
------------------------------------------------------------------------------ Start Your Social Network Today - Download eXo Platform Build your Enterprise Intranet with eXo Platform Software Java Based Open Source Intranet - Social, Extensible, Cloud Ready Get Started Now And Turn Your Intranet Into A Collaboration Platform http://p.sf.net/sfu/ExoPlatform
_______________________________________________ pocl-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/pocl-devel
