[PATCH] D72299: [DSE] Improve mayThrowBetween for 2 instructions in the same BB.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1644
+    return BlockMap->second[SI] !=
+           BlockMap->second[&*std::prev(NI->getIterator())];
+  }
----------------
efriedma wrote:
> What happens if NI is the first instruction in the block?  Or is that impossible in this context?
I think that should be impossible, as SI must come before NI (they cannot be equal either) and SI and NI must be in the same BB.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1706
+  // between them.
+  DenseMap<BasicBlock *, DenseMap<Instruction *, unsigned>> ThrowingBlocks;
 
----------------
efriedma wrote:
> This map seems sort of expensive, but maybe not a big deal relative to other stuff we have here already.
Yep, we probably should populate it lazily.  And I think at least for C/C++ programs, it should not really be needed at all. There's no difference in the MultiSource/SPEC2000/SPEC2006 binaries with and without this patch, now that I updated D72146 to skip throwing instructions modeled in MemorySSA.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72299