[PATCH] D112041: [InferAddressSpaces] Support assumed addrspaces from addrspace predicates.

Tue Oct 19 15:26:01 PDT 2021

hliao marked 4 inline comments as done.
hliao added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:196
   void inferAddressSpaces(ArrayRef<WeakTrackingVH> Postorder,
-                          ValueToAddrSpaceMapTy *InferredAddrSpace) const;
+                          ValueToAddrSpaceMapTy *InferredAddrSpace,
+                          PredicatedAddrSpaceMapTy &PredicatedAS) const;
----------------
tra wrote:
> I think this could've been a reference, too.
yeah, but we should address that in another change.

================
Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:903-905
 Optional<unsigned> InferAddressSpacesImpl::updateAddressSpace(
-    const Value &V, const ValueToAddrSpaceMapTy &InferredAddrSpace) const {
+    const Value &V, const ValueToAddrSpaceMapTy &InferredAddrSpace,
+    PredicatedAddrSpaceMapTy &PredicatedAS) const {
----------------
tra wrote:
> I can't say I'm happy about the way updateAddressSpace updates PredicateAS here, but delegates updates to InferredAddrSpace to the caller. I think both should be updated in one place -- either here, or in the callee.
the difference is that, here, we assume a use of pointer could be inferred with a new addrspace. The original is on a def of value.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112041/new/

https://reviews.llvm.org/D112041