[PATCH] D103009: [DSE] Eliminate memset after malloc

[Dawid Jurczak via Phabricator via llvm-commits]

yurai007 added a comment.

> And remove transformation from instcombine?

Yes. IIRC it's already dead because malloc has always more than one use. Could be done in separate change.

In D103009#2783897 <https://reviews.llvm.org/D103009#2783897>, @asbirlea wrote:

> This looks good to me, but please wait for the reviewers' comments from the previous patch version.
>
> I'd be curious how much the decision to not ignore allocas will affect compile times in (generated) IRs with many locals.

If this is about mallocs and callocs then impact on compile time is rather insignificant. I wouldn't expect anything more than noise but of course we can try to measure it to make sure.
Probably most time consuming part is in case of malloc - one AA check (AA.getModRefInfo) coming from memoryIsNotModifiedBetween and for calloc case that would be one getClobberingMemoryAccess call from MemorySSA (a little bit heavier then getModRefInfo). 
So it's more or less about one extra AA check for every pair of m(c)alloc-memset in block.



================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1801
+            cast<CallBase>(DefUOInst)->getOperand(0) == MemSet->getLength()) {
+          auto *Malloc = cast<CallInst>(DefUOInst);
+          if (DT.dominates(DefUOInst, cast<Instruction>(MemSet)) &&
----------------
That's root cause of some crashes. Cannot assume that isMallocLikeFn imply malloc. Will fix.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1810
+                                          Malloc->getArgOperand(0),
+                                          Malloc->getAttributes(), IRB, TLI)) {
+              Malloc->replaceAllUsesWith(Calloc);
----------------
xbolva00 wrote:
> We should not copy attributes from malloc.
Thanks for pointing this out! It saves me time of investigating current backend error from CI.
My understanding is that emitCalloc merge input attribute list with default calloc attributes.
If so then passing empty list should be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103009