[PATCH] D120586: [Attributor] Add AAAddressSpace to deduce address spaces

[Shilei Tian via Phabricator via llvm-commits]

tianshilei1992 added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:12104-12105
+      if (auto *SI = dyn_cast<StoreInst>(Inst))
+        if (SI->getPointerOperand() == AssociatedValue) {
+          MakeChange(Inst, const_cast<Use &>(U));
+          return true;
----------------
jdoerfert wrote:
> This doesn't actually help as it does not filter `store ptr %p, ptr %p`. Again, if we can just change both uses, then let's do so. If the verifier is unhappy, check that the use `U` is the pointer operand.
Why does the AS of value operand matter here?


================
Comment at: llvm/test/Transforms/Attributor/value-simplify.ll:804
+; TUNIT-NEXT:    [[TMP1:%.*]] = addrspacecast ptr addrspacecast (ptr addrspace(3) @ConstAS3Ptr to ptr) to ptr addrspace(3)
+; TUNIT-NEXT:    store i32 0, ptr addrspace(3) [[TMP1]], align 4
 ; TUNIT-NEXT:    ret void
----------------
jdoerfert wrote:
> tianshilei1992 wrote:
> > This should be replaced by `@ConstAS3Ptr` directly.
> Did you update the test, above it seems to work.
I expect it to be `store i32 0, ptr addrspace(3) @ConstAS3Ptr, align 4` directly but actually here it is more complicated than I thought.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120586