Hi Pekka,
thank you for the reply and for the paper, I have never heard of SAP before,
very interesting field.
Hi Peter,
the suggestion you gave my are what I am looking for, thank you very much.
The problem was the usage of tabs instead of spaces, fixed.
-
Similarly, your patch reformats parts of the Clang source code in a
way that is inconsistent with the coding standards. Please remove
these changes.
-
For such a large change as this, it would be really nice (and
easier to review) if your patch were broken up into a series of
separate logical changes. This is easy to do using git; I saw
you’re using hg so this should also be possible.
Ok, I’ll look for it in Mercurial.
- You added an OpenCLMask field to Qualifiers for storing the OpenCL
address space. This doubles the size of the Qualifiers object.
Is there any reason why you did not use the existing AddrSpace
subfield? (The way I imagine it could work is to define fixed
address space constants for the constant, global, local and private
address spaces).
I added a new mask for the sake of simplicity, but I think I’ll follow your advice.
You also don’t need to use a bitmask (at least
at the AST level) since the address spaces are mutually exclusive.
This is a problem on which I thought a lot. Since this qualifiers are mutually
exclusive I could have used only two bits to save one of the possible four cases.
But let’s take into consideration this test case:
typedef __global int global_typedef;
__local global_typedef* t4; //expected-error {{multiple named address spaces}}
The compilation error is due to the presence of a different OpenCL qualifier both
in the typdef both in the variable definition. Now, what is the canonical type of “t4” ?
“__global __local int”.
In the method Sema::HandleDeclarator there is a call to:
TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
GetTypeForDeclarator will build the canonical type of the declarator passed
as argument (D).
The actual stack trace is:
HandleDeclarator
GetTypeForDeclarator
BuildPointerType
getPointerType
getCanonicalType
The call to getCanonicalType will result in the building of a new “Qualifiers” object,
starting from “__local”, the local OpenCL qualifier the “__global” qualifier
will also be added.
If I add an assert in the Qualifier class to the method that adds the new qualifier (__global):
assert (there are no previous OpenCL qualifiers here)
it will for sure signal an error, since the Qualifiers object contains already an OpenCL
qualifiers: (__local).
To overcome this I allowed the possibility to have an inconsistent state in the Qualifiers
object, ie 2 bits in the OpenCL qualifiers mask, with the assurance that this inconsistency
would be checked in the method Sema::ActOnVariableDeclaration, also called by
HandleDeclarator.
One solution would be to add a check for duplicated OpenCL qualifiers in
HandleDeclarator before the call to GetTypeForDeclarator, but I think that this would be
misplaced since all the semantic checks on a variable declaration should be performed in
ActOnVariableDeclarator.
Right now I have no idea how to solve this issue.
Any suggestion is greatly appreciated.
- You added a __kernel field to FunctionDecl. A better way
of handling this IMO would be to add a new attribute (Attr subclass).
See the include/clang/Basic/Attr.td file.
Sorry for the stupid question, but what is the advantage of using an
attribute instead of a function specifier ?
Thank you very much again.
Bye bye.
Alberto