<div class="gmail_quote">On Fri, Oct 14, 2011 at 6:17 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 class="im">On Fri, Oct 14, 2011 at 05:11:31PM -0700, Justin Holewinski wrote:<br>
> Attached is an updated patch.  Any more comments before I commit?<br>
<br>
</div>I have just realised that OpenCL C should use its own TBAA root,<br>
because if we link OpenCL C and C code into the same module, the<br>
OpenCL C types may-alias the C types.  The C TBAA root should also<br>
be used for address spaces outside of the range of defined OpenCL<br>
address spaces (the implementation may use them), so that all unknown<br>
address spaces may-alias.<br></blockquote><div><br></div><div>Makes sense.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
> +  for (unsigned i = 0; i < LangAS::Count+1; ++i) {<br>
> +    CharCache[i] = 0;<br>
> +  }<br>
<br>
For the reasons above, this should actually be LangAS::Count+2.<br>
<br>
> +    // If we are in OpenCL mode, add an address space identifier to the name.<br>
> +    if (Context.getLangOptions().OpenCL) {<br>
> +      switch (Quals.getAddressSpace()) {<br>
> +      default:<br>
> +        llvm_unreachable("Unknown OpenCL address space");<br>
<br>
This is not unreachable if the implementation uses an unknown address<br>
space.  If we see this, we should use C TBAA's "onmipotent char".<br>
<br>
> +      case 0:<br>
> +        Name += " - OCL default";<br>
> +        break;<br>
> +      case LangAS::opencl_global:<br>
> +        Name += " - OCL global";<br>
> +        break;<br>
> +      case LangAS::opencl_local:<br>
> +        Name += " - OCL local";<br>
> +        break;<br>
> +      case LangAS::opencl_constant:<br>
> +        Name += " - OCL constant";<br>
> +        break;<br>
> +      }<br>
> +    }<br>
> +    CharCache[CacheIndex] = getTBAAInfoForNamedType(Name, getRoot());<br>
> +  }<br>
> +  return CharCache[CacheIndex];<br>
> +}<br>
> +<br>
> +unsigned CodeGenTBAA::getCacheIndex(Qualifiers Quals) {<br>
> +  unsigned AS = 0;<br>
> +  if (Context.getLangOptions().OpenCL) {<br>
> +    AS = Quals.getAddressSpace();<br>
> +    if (AS < LangAS::Offset || AS >= LangAS::Offset + LangAS::Count)<br>
> +     return AS;<br>
<br>
You should return 1 here, and add a case for address space 0 (in<br>
which case you should return 0).  Otherwise the function may return<br>
a value greater than LangAS::Count, and address spaces 1-4 would<br>
incorrectly may-alias the OpenCL address spaces (and may-not-alias<br>
address space 0).<br>
<br>
> +    else<br>
> +      return AS - LangAS::Offset;<br>
<br>
This should be AS - LangAS::Offset + 2.<br>
<br>
> +  }<br>
> +  return AS;<br>
>  }<br>
><br>
>  /// getTBAAInfoForNamedType - Create a TBAA tree node with the given string<br>
> @@ -94,16 +133,24 @@<br>
>    return false;<br>
>  }<br>
><br>
> -llvm::MDNode *<br>
> -CodeGenTBAA::getTBAAInfo(QualType QTy) {<br>
> +llvm::MDNode *CodeGenTBAA::getTBAAInfo(QualType QTy) {<br>
>    // If the type has the may_alias attribute (even on a typedef), it is<br>
>    // effectively in the general char alias class.<br>
>    if (TypeHasMayAlias(QTy))<br>
> -    return getChar();<br>
> +    return getChar(QTy.getQualifiers());<br>
> +  else<br>
> +    return getTBAAInfo(QTy.split());<br>
<br>
This should be QTy.getCanonicalType().split(), to avoid recomputing<br>
the canonical type in the function below.<br>
<br>
> +}<br>
><br>
> -  const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();<br>
> +llvm::MDNode *CodeGenTBAA::getTBAAInfo(SplitQualType QTy) {<br>
> +  // Retrieve the canonical type and the qualifiers for this QualType.<br>
> +  const Type *Ty = Context.getCanonicalType(QTy.first);<br>
<br>
You should just use QTy.first here.<br>
<br>
Other than that, go ahead.<br></blockquote><div><br></div><div>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.</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>