sizeof(long) in OpenCL C

The OpenCL C language requires that sizeof(long)=8, independent of the architecture. This is not what Clang currently implements.

How would I change this in clang? This issue is currently preventing supporting the "long" and (as a consequence) "double" type for 32-bit architectures in pocl <http://pocl.sourceforge.net/>.

I see that the file lib/Basic/TargetInfo.cpp defines e.g. "LongWidth = LongAlign = 32;". It seems that this definition should depend on the language chosen (-x cl) -- would this be the correct approach? What about all the targets that inherit from TargetInfo -- should they also receive the corresponding modification? How do I determine the current language in Clang?

Or would it be better to override these values after the TargetInfo object has been created? Is there already a language hook for this?

Any pointers, suggestions, or ideas for this would be appreciated.

-erik

Hi,

That approach would work: I did post a patch that included that, amongst
other OpenCL things, last year. However, it was felt that this wasn't an
area where the community wanted to put this in to the open source clang but
let different implementers do it according to their own designs in their own
code. If you do try this, you can use

if(Opts.OpenCL)....

to detect OpenCL. (If you look in clang there are certain uses of this to
deal with constant-folding/generating good error messages in cases where
OpenCL differs from C (mostly by being defined in more circumstances)).

Cheers,
Dave

Dave

Can you point me to your patch?

-erik

Found it: <http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20121119/068491.html>.

-erik

Could this be reconsidered? Is there a situation where one
does not want to adhere to the OpenCL C types when compiling
OpenCL C code? I do not understand the technical reasoning
behind the decision to not apply the patch.

Could this be reconsidered?

It looks more like the idea fizzled than was rejected. I also think
it's a good plan for anything that's specified by the language.

Tim.

Hi,

Apologies for not pointing Erik at the patch directly (annoyances of on
being in the UK time zone and all that...)

Could this be reconsidered?

It looks more like the idea fizzled than was rejected. I also think
it's a good plan for anything that's specified by the language.

My reading of things was that there was a general feeling that this wasn't
something the other OpenCL implementers felt belonged in open source clang,
so I didn't feel it was appropriate to push to change hearts and minds; but
maybe that has changed. Having said that, one thing that I think has
happened is that quite how OpenCL (and associated things like SPIR) split
between the open source clang/LLVM and implementers choices stuff is
something that might be something worthy of an actual discussion. (This is
particularly relevant since the clang frontend does a comparatively large
amount of code processing in order to detect and issue good diagnostics.) It
might be something that interested parties might want to get together to
discuss at the upcoming North American LLVM developer meeting: certainly if
there's an interest one of the ARM attendees would join the discussion and
try to help make the situation better.

Cheers,
Dave

Hi all,

I would like to join the discussion because I am interested in solving this
issue too. However my opinion is not in favor of the proposed patch (
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130812/086263.html
). Being the question target-independent, I don't see the
TargetInfo::setForcedLangOptions the correct place for two reasons:
1. in CompilerInstance.cpp : 681-682 shows that having mutable target info is
not good
2. the method TargetInfo::setForcedLangOptions now is declared *virtual*,
allowing derived classes to overload it, making the the enforcement of language
options weak.

If (1) is still considered true, then TargetInfo::setForcedLangOptions should be
removed.
A possible way to achieve the same flexibility would be to have a wrapper class
"LanguageInfo" that handles the TargetInfo instance. This class would expose
methods to have knowledge about type size and alignment (like the TargetInfo
class) and whatever info that can be either target-dependent or not depending on
the language. These methods would simply check the current language
configuration in LangOptions and would redirect the query to the TargetInfo
class if no language enforcement is required otherwise will return the enforced
information.

If (1) is considered a too much strong requirement then the overloading of
TargetInfo::setForcedLangOptions should be avoided.

What do you think about this?

Thanks in advance.

Regards,
-Michele

Likewise I'm very interested in any other parties' contributions to this.
Comments inline:

However, it was felt that this wasn't an
area where the community wanted to put this in to the open source clang

but

let different implementers do it according to their own designs in their

own

code.

Could this be reconsidered? Is there a situation where one
does not want to adhere to the OpenCL C types when compiling
OpenCL C code? I do not understand the technical reasoning
behind the decision to not apply the patch.

I would like to join the discussion because I am interested in solving

this

issue too. However my opinion is not in favor of the proposed patch (

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130812/086263.h
tml

). Being the question target-independent, I don't see the
TargetInfo::setForcedLangOptions the correct place for two reasons:
1. in CompilerInstance.cpp : 681-682 shows that having mutable target info

is

not good
2. the method TargetInfo::setForcedLangOptions now is declared *virtual*,
allowing derived classes to overload it, making the the enforcement of

language

options weak.

If (1) is still considered true, then TargetInfo::setForcedLangOptions

should be

removed.

I was reading those comments a slightly different way: that this processing
should be performed at target-info object construction time (possibly by
calling
setForcedLangOptions), and any usage
that involves mutating the target info (eg, conceivably one might get that
in a
"compiler daemon" that's jumping between languages) should be modified in
order not to mutate
a target info. The second point is a bigger issue, particularly since a
target
could (if it had some peculiar desire to) validly increase the alignment
specification of an OpenCL type, but isn't allowed by the spec to decrease
it.
Enforcing those kind of constraints looks to be a bit of a nightmare.

A possible way to achieve the same flexibility would be to have a wrapper

class

"LanguageInfo" that handles the TargetInfo instance. This class would

expose

methods to have knowledge about type size and alignment (like the

TargetInfo

class) and whatever info that can be either target-dependent or not

depending on

the language. These methods would simply check the current language
configuration in LangOptions and would redirect the query to the

TargetInfo

class if no language enforcement is required otherwise will return the

enforced

information.

This sounds reasonable, with my only reservation that given the relatively
few
forced language options (OpenCL's types+alignments combined with a couple of
oddball
things) it might be considered a bit too much abstraction for the practical
poblem.
Are there likely to be any other C family languages with anything specific
in
them added to clang in the future?

If (1) is considered a too much strong requirement then the overloading of
TargetInfo::setForcedLangOptions should be avoided.

I'm unsure on this one, see above.

Cheers,
Dave

Likewise I'm very interested in any other parties' contributions to this.
Comments inline:

However, it was felt that this wasn't an
area where the community wanted to put this in to the open source clang

but

let different implementers do it according to their own designs in their

own

code.

Could this be reconsidered? Is there a situation where one
does not want to adhere to the OpenCL C types when compiling
OpenCL C code? I do not understand the technical reasoning
behind the decision to not apply the patch.

> I would like to join the discussion because I am interested in solving
this
> issue too. However my opinion is not in favor of the proposed patch (
>
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130812/086263.h
tml
> ). Being the question target-independent, I don't see the
> TargetInfo::setForcedLangOptions the correct place for two reasons:
> 1. in CompilerInstance.cpp : 681-682 shows that having mutable target info
is
> not good
> 2. the method TargetInfo::setForcedLangOptions now is declared *virtual*,
> allowing derived classes to overload it, making the the enforcement of
language
> options weak.

> If (1) is still considered true, then TargetInfo::setForcedLangOptions
should be
> removed.

I was reading those comments a slightly different way: that this processing
should be performed at target-info object construction time (possibly by
calling
setForcedLangOptions), and any usage
that involves mutating the target info (eg, conceivably one might get that
in a
"compiler daemon" that's jumping between languages) should be modified in
order not to mutate
a target info. The second point is a bigger issue, particularly since a
target
could (if it had some peculiar desire to) validly increase the alignment
specification of an OpenCL type, but isn't allowed by the spec to decrease
it.
Enforcing those kind of constraints looks to be a bit of a nightmare.

> A possible way to achieve the same flexibility would be to have a wrapper
class
> "LanguageInfo" that handles the TargetInfo instance. This class would
expose
> methods to have knowledge about type size and alignment (like the
TargetInfo
> class) and whatever info that can be either target-dependent or not
depending on
> the language. These methods would simply check the current language
> configuration in LangOptions and would redirect the query to the
TargetInfo
> class if no language enforcement is required otherwise will return the
enforced
> information.

This sounds reasonable, with my only reservation that given the relatively
few
forced language options (OpenCL's types+alignments combined with a couple of
oddball
things) it might be considered a bit too much abstraction for the practical
poblem.
Are there likely to be any other C family languages with anything specific
in
them added to clang in the future?

It's just a proxy object to decouple language enforced option from target specific preference. Maybe a less natural but lightweight approach could be the following:
- a class LanguageInfo that contains infos related of enforced language options plus query methods to know is a given parameter must be enforced or not.
- the TargetInfo class, e.g. for type size and alignment, query to LanguageInfo instance if the size of a given type is enforced by the language, if positive it uses the value from the query otherwise it uses the target value.

In this way the modification are focused on the TargetInfo class implementation keeping the current use.

The flexibility of alignment for OpenCL types can be implemented using the LanguageInfo to have the language minimum alignment and using it to check if the target alignment is a multiple so to be sure that the target choice is not in conflict with the language constraints.

> If (1) is considered a too much strong requirement then the overloading of
> TargetInfo::setForcedLangOptions should be avoided.

I'm unsure on this one, see above.

If target informations are language dependent then I would prefer a method
like TargetInfo::updateLanguage instead of TargetInfo::setForcedLangOptions. In this way is explicit the fact that the TargetInfo is not immutable. But this is still not the place where to enforce language independent requirements because such a method must be virtual.

The alternative solution through a query system probably would be enough to fix the problem for OpenCL and similar cases and it would not need to check and fix all the uses of TargetInfo in the frontend.

Opinions?

Thanks in advance.

Regards,
-Michele

I'd like to throw a few more bones into the discussion. OpenCL C differs from C and/or C++ in a few other, type-related ways:

- char is always signed
- "signed char" does not exist
- "long long" (and "unsigned long long") does not exist

Would this all fit into the LanguageInfo idea? Would removing "signed char" also require modifying the parser?

-erik

I notice that, with the patch applied, "long" has 64 bits, but __INTPTR_TYPE__ is still defined as "long int" on ARM. This is now wrong; it needs to be set to "int" instead. Do you have pointers for correcting this in clang?

There are similar issues with e.g. __INT64_TYPE__ (should now be "long" instead of "long long")

-erik

I'd like to throw a few more bones into the discussion. OpenCL C differs from C and/or C++ in a few other, type-related ways:

- char is always signed

This one yes: is something that generally is target-dependent but in the case of OpenCL is fixed.

- "signed char" does not exist

I don't know if the OpenCL specification declares the 'signed' modifier not allowed. BTW, such check should be part of the type checking: if the parsed language is OpenCL then explicitly signed types must be rejected (or at least a warning must be emitted).

- "long long" (and "unsigned long long") does not exist

About "long long" and other similar types are considered "reserved" so I am not sure that is correct to produce an error.

Hi,

I notice that, with the patch applied, "long" has 64 bits, but

__INTPTR_TYPE__ is still defined as "long int" on ARM. This is now wrong; it
needs to be set to "int" instead. Do you have pointers for

correcting this in clang?

Unfortunately not; implementation (even such simple things as choice of
size) of pointers for OpenCL devices is one of those "implementations may
make different choices" areas where things don't map cleanly to the standard
clang front-end.

Cheers,
Dave

Specifying the size of “long” must have seemed clever to someone, but it fundamentally changes the target in some pretty intrusive ways, and frankly it ought to be represented somehow in the target triple and everything that depends on it. That is, Open CL targeting ARM is no longer just the ordinary ARM target, and a significant amount of code (e.g. the ABI lowering code in IR-gen) needs to be revisited to see whether it’s still applicable in the face of this change.

John.

Since OpenCL C code cannot access any system libraries, one is free to re-define the ABI in any way one sees fit (except that one must still be able to launch kernels). This opens avenues for optimization, as e.g. operating system constraints or backward compatibility are non-issues.

On the other hand, clang (the front-end) should not be concerned with ABIs, and the back-end (llvm) should not be concerned with whether a type is called "long" or "long long" in the language syntax. In other words, maybe this issue could be addressed by mapping OpenCL C's "long" to C's "long long" on 32-bit architectures?

-erik

Uhm.. Implementation defined types makes type size enforcing complex. If the mapping must be direct I think there is no solution.

If the relationship is just between implementation defined type and (sign, bitwidth) then the mapping can be done on the fly looking for the pair (sign, bitwidth) in the native types map, picking a compatible type.
The fact that there is no direct control on the mapping can be an issue.

-Michele

I have proposed an updated patch on cfe-commits.

-erik

Hi,

> I notice that, with the patch applied, "long" has 64 bits, but
__INTPTR_TYPE__ is still defined as "long int" on ARM. This is now wrong; it
needs to be set to "int" instead. Do you have pointers for
> correcting this in clang?

Unfortunately not; implementation (even such simple things as choice of
size) of pointers for OpenCL devices is one of those "implementations may
make different choices" areas where things don't map cleanly to the standard
clang front-end.

Uhm.. Implementation defined types makes type size enforcing complex. If the
mapping must be direct I think there is no solution.

What I meant was: pointers within the context of OpenCL may not necessarily be implemented in the same way as on the "underlying" target. As such, embedding an assumption that the pointer type is the same within the common clang code is probably the wrong way to do things to be useful as common code. (Of course there it may actually be the same for some ways of implementing OpenCL, just that it need not always be.) I think Erik's later patch introducing a distinct PointerWidth seems like a suitable mechanism.

If the relationship is just between implementation defined type and (sign,
bitwidth) then the mapping can be done on the fly looking for the pair (sign,
bitwidth) in the native types map, picking a compatible type.
The fact that there is no direct control on the mapping can be an issue.

I don't quite understand what you mean here; as above the key thing I was mentioning was that the size of a pointer used when working as a OpenCL compiler may be different from the one that would be chosen when working as a C compiler, so simply making the "C compiler" pointer width expressed in terms of types defined identically in OpenCL and C is not the way to go.

Cheers,
Dave

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782

Hi,

> I notice that, with the patch applied, "long" has 64 bits, but
__INTPTR_TYPE__ is still defined as "long int" on ARM. This is now wrong; it
needs to be set to "int" instead. Do you have pointers for
> correcting this in clang?

Unfortunately not; implementation (even such simple things as choice of
size) of pointers for OpenCL devices is one of those "implementations may
make different choices" areas where things don't map cleanly to the standard
clang front-end.

> Uhm.. Implementation defined types makes type size enforcing complex. If the
> mapping must be direct I think there is no solution.

What I meant was: pointers within the context of OpenCL may not necessarily be implemented in the same way as on the "underlying" target. As such, embedding an assumption that the pointer type is the same within the common clang code is probably the wrong way to do things to be useful as common code. (Of course there it may actually be the same for some ways of implementing OpenCL, just that it need not always be.) I think Erik's later patch introducing a distinct PointerWidth seems like a suitable mechanism.

> If the relationship is just between implementation defined type and (sign,
> bitwidth) then the mapping can be done on the fly looking for the pair (sign,
> bitwidth) in the native types map, picking a compatible type.
> The fact that there is no direct control on the mapping can be an issue.

I don't quite understand what you mean here; as above the key thing I was mentioning was that the size of a pointer used when working as a OpenCL compiler may be different from the one that would be chosen when working as a C compiler, so simply making the "C compiler" pointer width expressed in terms of types defined identically in OpenCL and C is not the way to go.

Picking 'size_t' as example: on a 64bit target I would expect that 'size_t' will
be mapped to an unsigned 64bit builtin integer type. If I declare LongWidth =
LongLongWidth = 64 then both 'unsigned long' and 'unsigned long long' would be
valid choice for 'size_t'. Expressing the relationship in a indirect way then
the definition of implementation defined types is not affected by the potential
changes that OpenCL language definition may impose respect to the choice done
for the C compiler.

BTW, it seems that the mapping cannot be just based on size and sign parameters.

Regards,
-Michele