arsenm added inline comments.
================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:227
+ Optional<unsigned> getAssumedAddrSpace(const Value *V) const {
+ return getTLI()->getTargetMachine().getAssumedAddrSpace(V);
----------------
hliao wrote:
> hliao wrote:
> > arsenm wrote:
> > > We already have a -1 as an invalid addrspace, so optional isn't necessary
> > OK. Do we need to document that in the IR langref? That invalid address
> > space ID is not documented anywhere.
> Shall we assume this before that reserved invalid address space ID is
> well-received and documented in LLVM IR ref? I will make either change in
> this change accordingly.
It's not really part of the IR. The address space is a 24-bit unsigned limit
and this is already the pass convention
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:540
+ const auto *Ptr = LD->getPointerOperand();
+ if (Ptr->getType()->getPointerAddressSpace() != AMDGPUAS::CONSTANT_ADDRESS)
+ return None;
----------------
hliao wrote:
> arsenm wrote:
> > I think this may be too aggressive and should check if it's specifically a
> > kernel argument load. Contant address space should also not be necessary
> If the variable locates in `__constant__` memory, it could only be modified
> on the host side. It's safe to assume any generic pointer loaded from
> `__constant__` memory is a global one.
I thought we discussed the one possible case where this wasn't necessarily true
================
Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:289-291
+ if (TTI->getAssumedAddrSpace(&V))
+ return true;
return false;
----------------
This can be return TII->...
================
Comment at:
llvm/test/Transforms/InferAddressSpaces/AMDGPU/assumed-addrspace.ll:12
+ ret float %v
+}
----------------
Should have test with load from kernel argument
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91121/new/
https://reviews.llvm.org/D91121
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
