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

[Dawid Jurczak via Phabricator via llvm-commits]

yurai007 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) &&
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103009