rjmccall added a comment. In D75423#1901348 <https://reviews.llvm.org/D75423#1901348>, @yaxunl wrote:
> In D75423#1901254 <https://reviews.llvm.org/D75423#1901254>, @rjmccall wrote: > > > In D75423#1901206 <https://reviews.llvm.org/D75423#1901206>, @hliao wrote: > > > > > invariant checking already takes account of loading from constant address > > > space or memory (AA::pointsToConstantMemory), that's almost equivalent to > > > adding invariant attributes. Why do we mark these constant loads with > > > additional attributes? > > > > > > If alias analysis already knows that all memory in the constant address > > space is immutable, I agree that should be sufficient, at least on targets > > where the constant address space is actually a different address space in > > LLVM IR. If it only knows that when it can resolve an access down to a > > specific `constant` global variable, this patch is still providing value > > because it applies even for accesses through a pointer. > > > There is plan for amdgcn target to drop constant address space in IR and use > global address space instead. > > Also, currently there are targets (e.g. x86_64) supporting OpenCL which does > not have constant address space in IR. Okay, then I have no problem taking a patch for this into IRGen. But I think it should be fine to do this by adding the invariant-load metadata when loading from an l-value instead of injecting invariant-groups into l-value emission. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75423/new/ https://reviews.llvm.org/D75423 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
