[cfe-commits] [PATCH/RFC] Extend TBAA to handle OpenCL address spaces

[Justin Holewinski]

On Fri, Oct 14, 2011 at 6:17 PM, Peter Collingbourne <peter at pcc.me.uk>wrote:

> On Fri, Oct 14, 2011 at 05:11:31PM -0700, Justin Holewinski wrote:
> > Attached is an updated patch.  Any more comments before I commit?
>
> I have just realised that OpenCL C should use its own TBAA root,
> because if we link OpenCL C and C code into the same module, the
> OpenCL C types may-alias the C types.  The C TBAA root should also
> be used for address spaces outside of the range of defined OpenCL
> address spaces (the implementation may use them), so that all unknown
> address spaces may-alias.
>

Makes sense.


>
> > +  for (unsigned i = 0; i < LangAS::Count+1; ++i) {
> > +    CharCache[i] = 0;
> > +  }
>
> For the reasons above, this should actually be LangAS::Count+2.
>
> > +    // If we are in OpenCL mode, add an address space identifier to the
> name.
> > +    if (Context.getLangOptions().OpenCL) {
> > +      switch (Quals.getAddressSpace()) {
> > +      default:
> > +        llvm_unreachable("Unknown OpenCL address space");
>
> This is not unreachable if the implementation uses an unknown address
> space.  If we see this, we should use C TBAA's "onmipotent char".
>
> > +      case 0:
> > +        Name += " - OCL default";
> > +        break;
> > +      case LangAS::opencl_global:
> > +        Name += " - OCL global";
> > +        break;
> > +      case LangAS::opencl_local:
> > +        Name += " - OCL local";
> > +        break;
> > +      case LangAS::opencl_constant:
> > +        Name += " - OCL constant";
> > +        break;
> > +      }
> > +    }
> > +    CharCache[CacheIndex] = getTBAAInfoForNamedType(Name, getRoot());
> > +  }
> > +  return CharCache[CacheIndex];
> > +}
> > +
> > +unsigned CodeGenTBAA::getCacheIndex(Qualifiers Quals) {
> > +  unsigned AS = 0;
> > +  if (Context.getLangOptions().OpenCL) {
> > +    AS = Quals.getAddressSpace();
> > +    if (AS < LangAS::Offset || AS >= LangAS::Offset + LangAS::Count)
> > +     return AS;
>
> You should return 1 here, and add a case for address space 0 (in
> which case you should return 0).  Otherwise the function may return
> a value greater than LangAS::Count, and address spaces 1-4 would
> incorrectly may-alias the OpenCL address spaces (and may-not-alias
> address space 0).
>
> > +    else
> > +      return AS - LangAS::Offset;
>
> This should be AS - LangAS::Offset + 2.
>
> > +  }
> > +  return AS;
> >  }
> >
> >  /// getTBAAInfoForNamedType - Create a TBAA tree node with the given
> string
> > @@ -94,16 +133,24 @@
> >    return false;
> >  }
> >
> > -llvm::MDNode *
> > -CodeGenTBAA::getTBAAInfo(QualType QTy) {
> > +llvm::MDNode *CodeGenTBAA::getTBAAInfo(QualType QTy) {
> >    // If the type has the may_alias attribute (even on a typedef), it is
> >    // effectively in the general char alias class.
> >    if (TypeHasMayAlias(QTy))
> > -    return getChar();
> > +    return getChar(QTy.getQualifiers());
> > +  else
> > +    return getTBAAInfo(QTy.split());
>
> This should be QTy.getCanonicalType().split(), to avoid recomputing
> the canonical type in the function below.
>
> > +}
> >
> > -  const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
> > +llvm::MDNode *CodeGenTBAA::getTBAAInfo(SplitQualType QTy) {
> > +  // Retrieve the canonical type and the qualifiers for this QualType.
> > +  const Type *Ty = Context.getCanonicalType(QTy.first);
>
> You should just use QTy.first here.
>
> Other than that, go ahead.
>

I'm not going to have this done before the branch, so I'll just hold off on
it until we can spend more time on it after the branch.


>
> Thanks,
> --
> Peter
>



-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111014/7ba09157/attachment.html>