[PATCH] D136201: [InstCombine] Handle PHI nodes when eliminating constant memcpy
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 13 15:17:39 PST 2022
arsenm added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:269
+ // Check if any of the incoming values of PHI is the destination of Copy
+ auto CopySrcAddrSpace = Copy->getSourceAddressSpace();
+ auto PHIAddrSpace = PHI->getType()->getPointerAddressSpace();
----------------
Don't like auto for addrspace
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:111
MemTransferInst *MI = dyn_cast<MemTransferInst>(I);
- if (!MI)
+ if (!MI || isa<PHINode>(MI->getDest()))
return false;
----------------
gandhi21299 wrote:
> nikic wrote:
> > This is not sufficient: E.g. consider doing the memcpy to an addrspacecast of the phi or similar. I think what we'd need instead is that set `IsOffset=true` for phi nodes, so we'll recursively reject memcpys into it. Of course we would need a different name than IsOffset in that case. It would more generally indicate that the pointer is not equivalent to the base.
> @nikic What do you mean by "need a different name"? I could simply emplace_back a pair of the instruction and a true to `ValuesToInspect` vector right?
I think I'm getting lost in the different revisions of the patch; not sure what this comment is referring to anymore. There isn't anything about phi handling right here?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136201/new/
https://reviews.llvm.org/D136201
More information about the llvm-commits
mailing list