This is an archive of the discontinued LLVM Phabricator instance.

[DeadStoreElimination] Remove dead zero store to calloc initialized memory
ClosedPublic

Authored by igor-laevsky on Sep 21 2015, 6:53 AM.

Details

Summary

This change allows dead store elimination to remove zero and null stores into memory freshly allocated with calloc-like function.

Philip, this is variation of your old change (http://reviews.llvm.org/D3942). Hopefully all issues with msan should be fixed.

Diff Detail

Repository
rL LLVM

Event Timeline

igor-laevsky retitled this revision from to [DeadStoreElimination] Remove dead zero store to calloc initialized memory.
igor-laevsky updated this object.
igor-laevsky added a reviewer: reames.
igor-laevsky set the repository for this revision to rL LLVM.
igor-laevsky added a subscriber: llvm-commits.
reames edited edge metadata.Sep 21 2015, 8:18 AM

Mostly looks pretty good. Minor, but required, comments.

lib/Transforms/Scalar/DeadStoreElimination.cpp
535 ↗(On Diff #35251)

This should be outside the surrounding loop.

537 ↗(On Diff #35251)

The code here looks correct, but I'm a bit concerned about compile time impact. GetUnderlyingObject might be fairly slow (since it has to chase through GEPs and bitcasts). Possibly you should check to see if you have an appropriate constant before executing this query?

663 ↗(On Diff #35251)

The rename looks correct, but I haven't reviewed the correctness of the existing code.

test/Transforms/DeadStoreElimination/calloc-store.ll
9 ↗(On Diff #35251)

Please add a couple of negative tests, specifically:

  • base object is not a calloc function
  • memory is clobbered
  • store is volatile (i.e. not removable)
  • store of wrong constant
  • store of non-constant
igor-laevsky edited edge metadata.
igor-laevsky marked 3 inline comments as done.
reames accepted this revision.Sep 22 2015, 3:39 PM
reames edited edge metadata.

LGTM w/one minor comment.

test/Transforms/DeadStoreElimination/calloc-store.ll
46 ↗(On Diff #35264)

Stale comment.

This revision is now accepted and ready to land.Sep 22 2015, 3:39 PM
This revision was automatically updated to reflect the committed changes.