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

[Peter Collingbourne]

On Fri, Oct 14, 2011 at 03:31:43PM -0700, Justin Holewinski wrote:
> On Fri, Oct 14, 2011 at 11:57 AM, Peter Collingbourne <peter at pcc.me.uk>wrote:
> 
> > On Fri, Oct 14, 2011 at 02:01:46PM -0400, Justin Holewinski wrote:
> > > -llvm::MDNode *CodeGenTBAA::getChar() {
> > > +llvm::MDNode *CodeGenTBAA::getChar(Qualifiers& Quals) {
> >
> > Qualifiers is a lightweight type, and should be passed by value.
> >
> > >  llvm::MDNode *
> > > -CodeGenTBAA::getTBAAInfo(QualType QTy) {
> > > +CodeGenTBAA::getTBAAInfo(QualType QTy, Qualifiers *Override) {
> >
> > I think it's best if you split getTBAAInfo into two functions:
> > one which takes a QualType and the other which takes a canonical
> > SplitQualType.  The former would check for the may_alias attribute,
> > while the latter would contain the rest of the code (and be called
> > recursively for the unsigned->signed mapping).  To call the latter from
> > the former, you can obtain a SplitQualType using the QualType::split
> > function on the canonical type.
> >
> > > @@ -42,10 +43,12 @@
> > >    MangleContext &MContext;
> > >
> > >    /// MetadataCache - This maps clang::Types to llvm::MDNodes describing
> > them.
> > > -  llvm::DenseMap<const Type *, llvm::MDNode *> MetadataCache;
> > > +  llvm::DenseMap<std::pair<const Type *, unsigned>, llvm::MDNode *>
> > > +    MetadataCache;
> > >
> > >    llvm::MDNode *Root;
> > > -  llvm::MDNode *Char;
> > > +
> > > +  llvm::DenseMap<unsigned, llvm::MDNode *> CharCache;
> >
> > A cheaper way of defining MetadataCache and CharCache would be
> > as arrays of (respectively) DenseMaps or MDNode pointers of size
> > LangAS::Count+1.  You can then define a function which defines
> > a mapping from a Qualifiers object to an integer between 0 and
> > LangAS::Count (the return value of which conceptually represents a
> > set of aliasing qualifiers) which would be used to index the arrays.
> >
> 
> Thanks for the comments.  Are you okay with the general approach?

Yes.  If you make the changes I mentioned, and add a test case,
I think it's OK to commit.

Thanks,
-- 
Peter