[polly] r256650 - IslExprBuilder: Provide PointerLikeTypeTraits for isl_id
Tobias Grosser via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 30 23:30:33 PST 2015
On 12/30/2015 10:17 PM, Chandler Carruth wrote:
> On Wed, Dec 30, 2015 at 12:15 PM Tobias Grosser via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
> Author: grosser
> Date: Wed Dec 30 14:11:48 2015
> New Revision: 256650
>
> URL: http://llvm.org/viewvc/llvm-project?rev=256650&view=rev
> Log:
> IslExprBuilder: Provide PointerLikeTypeTraits for isl_id
>
> Providing an explicit PointerLikeTypeTraits implementation became
> necessary
> since LLVM started in
> https://llvm.org/svn/llvm-project/llvm/trunk@256620
> to automatically derive the pointer alignment from the pointer
> element type,
> which does not work for incomplete types as used by isl. To ensure
> our code
> still compiles, we provide an instantiation of PointerLikeTypeTraits
> for isl_id
> which assumes no minimal alignment. isl pointers are likely to have
> a "higher"
> alignment. We can exploit this later in case this becomes
> performance relevant.
>
> Modified:
> polly/trunk/include/polly/CodeGen/IslExprBuilder.h
>
> Modified: polly/trunk/include/polly/CodeGen/IslExprBuilder.h
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/CodeGen/IslExprBuilder.h?rev=256650&r1=256649&r2=256650&view=diff
> ==============================================================================
> --- polly/trunk/include/polly/CodeGen/IslExprBuilder.h (original)
> +++ polly/trunk/include/polly/CodeGen/IslExprBuilder.h Wed Dec 30
> 14:11:48 2015
> @@ -23,6 +23,21 @@ class DataLayout;
> class ScalarEvolution;
> }
>
> +struct isl_id;
> +
> +namespace llvm {
> +// Provide PointerLikeTypeTraits for isl_id.
> +template <> class PointerLikeTypeTraits<isl_id *> {
>
>
> Unfortunately, this almost guarantees an ODR violation if anyone ever
> causes an instantiation of the primary template without having included
> this header.
Yes, I was afraid of such kind of issues here.
My reasoning was that in practice this might be pretty save (but rather
ugly).
I found this piece of C++ on stackoverflow:
======================
ยง14.7.3 [temp.expl.spec]/p6 (emphasis added):
If a template, a member template or a member of a class template is
explicitly specialized then that specialization shall be declared before
the first use of that specialization that would cause an implicit
instantiation to take place, in every translation unit in which such a
use occurs; no diagnostic is required. If the program does not provide a
definition for an explicit specialization and either the specialization
is used in a way that would cause an implicit instantiation to take
place or the member is a virtual member function, the program is
ill-formed, no diagnostic required.
======================
As we do not have a definition of isl_id available outside of isl
itself, an instantiation of a PointerLikeTypeTraits without the
specialization above results in a compile time error. So we would always
be warned if this instantiation would be needed, but never accidentally
instantiate a generic version of this template for isl_id.
> Why does isl_id need to remain incomplete here?
Because the type of isl_id is not exposed in any of the public headers
of isl. I could probably include private headers, but this would require
to make such headers available by adding additional include paths, ...
As they are private on purpose, this seems to be just a bad hack.
> If it truly needs to be opaque, I would suggest changing whatever data
> structure to explicitly operate on void* which we provide a generic
> template for which assumes at least 4-byte alignment. This template is
> intended precisely for the cases where the types of the pointers change,
> aren't possibly known ahead of time, etc., and the code actually must
> erase the type and make generic assumptions.
>
> I find using void* to do this somewhat more transparent. What do you think?
This would mean we loose static type checking, clang auto-completion,
and the code becomes also more difficult to read as we would need to
document what void* actually means.
Finally, as stated in your other email, we can theoretically not even
rely on the pointer to be 4 bit aligned. (This is why I set the number
of free bits to 0).
I was thinking to move to standard C++ data structures, but
unfortunately they do not provide a MapVector. This solution - despite
not very beautiful - seems to work and be reasonably save. In the
optimal case I would hope that DenseMap/MapVector/... could be used
without them exploiting pointer alignment. Let's keep this discussion in
the other thread.
Best,
Tobias
More information about the llvm-commits
mailing list