[PATCH] D147025: [InstCombine] Teach alloca replacement to handle `addrspacecast`

[Michael Liao via Phabricator via llvm-commits]

hliao added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:279
+    const auto *ASC = dyn_cast<AddrSpaceCastInst>(I);
+    return ASC && IC.isValidAddrSpaceCast(FromAS, ASC->getDestAddressSpace());
+  }
----------------
yaxunl wrote:
> Do we really need to check whether the addrspacecast is valid here?
> 
> If it is invalid, then it is UB. Therefore we could just assume it is valid.
> 
> According to the LLVM IR manual, we can assume the memory pointed to by addrspacecasted pointers are the same, therefore we should be able to do this optimization always.
> 
> Another reason is that there should already be validity check for addrspacecast in some target specific passes, therefore there is no need to check it here.
We should not transform valid code with well-defined behavior into one with UB. It's valid for the developer to copy data from an address space (not eligible to be cast into generic address space) into private memory and pass that data into a routine accepting generic pointers only. For this kind of case, we should not transform that code or generate a UB address space cast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147025