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

<br>
Thanks,<br>
<font color="#888888">--<br>
Peter<br>
</font></blockquote></div><br><br clear="all"><div><br></div>-- <br><br><div>Thanks,</div><div><br></div><div>Justin Holewinski</div><br>