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 <[email protected] > <mailto:[email protected]>> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits -- Pekka _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
