[PATCH] D95019: [ConstantHoisting] Fix bug where constant materialization could insert into EH pad

Thu Jan 21 13:33:15 PST 2021

Holman added a comment.

In D95019#2513466 <https://reviews.llvm.org/D95019#2513466>, @rnk wrote:

> Out of curiosity, have you bisected this to a particular change?

I haven't. It was crashing LTO build so repro was time consuming. I was trying to find recent changes that might have allowed more hoisting to happen, but there wasn't anything obvious to me. We took a compiler drop around 11/25 and then the next one we took on 1/3 was failing.

================
Comment at: llvm/lib/Transforms/Scalar/ConstantHoisting.cpp:190
+  if (Idx != ~0U && isa<PHINode>(Inst)) {
+    auto InsertionBlock = cast<PHINode>(Inst)->getIncomingBlock(Idx);
+    if (!InsertionBlock->isEHPad()) {
----------------
rnk wrote:
> Are you sure you meant to shadow the outer variable here? InsertionBlock will be null in the case you are trying to handle, a phi, preceded by a catchswitch, and I'm not sure what `DT->getNode(nullptr)` returns.
Good catch, thanks! I originally just copy/pasted the EH loop when I was testing and must have introduced that bug when I refactored.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95019