[PATCH] D103009: [DSE] Transform memset + malloc --> calloc (PR25892)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1843
+            return false;
+          if (Malloc->getOperand(0) == MemSet->getLength()) {
+            if (DT.dominates(DefUOInst, MemSet) &&
----------------
yurai007 wrote:
> xbolva00 wrote:
> > We dont need this check, do we?
> > 
> > p = malloc(20)
> > memset(p, 0, 10)
> > 
> > Reading p between 10 and 20 is UB, so with calloc we would have 0s in this area so fine.
> > 
> > And reverse case is UB too.
> If we permitted to "calloc more than we memset" wouldn't we hide UB in some cases?
> Like if we would really read unitinitialized memory much later after memset?
> The second thing is that GCC doesn't transform malloc to calloc when we memset less memory than malloc allocates: https://godbolt.org/z/Ef94je4KP I'm not saying we should blindly follow them, I'm just not sure what was rationale behind that.
If the memset size is different, profitability is also unclear. Converting a malloc into a calloc may be always legal, but if you have malloc(10000) followed by memset(10) it's probably not profitable to actually do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103009