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

[Pekka Jääskeläinen]

Hi Justin,

Has there been any further progress with the TBAA for the OpenCL
disjoint address spaces?

On 10/15/2011 04:34 AM, Justin Holewinski wrote:
> On Fri, Oct 14, 2011 at 6:17 PM, Peter Collingbourne <peter at pcc.me.uk
> <mailto: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
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


-- 
Pekka