[cfe-dev] sizeof(long) in OpenCL C

[Michele Scandale]

On 08/30/2013 03:40 PM, David Tweed wrote:
> Likewise I'm very interested in any other parties' contributions to this.
> Comments inline:
>
> On 08/29/2013 10:40 PM, Pekka Jääskeläinen wrote:
>> On 08/29/2013 07:44 PM, David Tweed wrote:
>>> 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