[PATCH] D102153: [SelectionDAG] Fix argument copy elision with irregular types

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:9824
-        DL.getTypeStoreSize(Arg->getType()) !=
-            DL.getTypeAllocSize(AI->getAllocatedType()) ||
         ArgCopyElisionCandidates.count(Arg)) {
----------------
LemonBoy wrote:
> rnk wrote:
> > Don't we still need this check? I don't see any other code that checks that the allocated type and argument type match. Otherwise we could try to copy elide an i32 stored into an i64 alloca, for example. Please add a test for that, sorry for not already having one.
> In `tryToElideArgumentCopy` there's a check between the object size pointed by the old and new frame indices, in case of a partially initialized `i64` being forwarded as a `i32` the check will fail.
> No strong feeling about this, I can put the check back if you prefer.
I think we should keep the store size vs. alloca size check here. If I were debugging this code later, I would be confused as to how an alloca like this became a copy elision candidate that gets rejected later in tryToElideArgumentCopy.


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

https://reviews.llvm.org/D102153