[PATCH] D126650: [InstCombine] Fix const folding of switched with default case

Danila Malyutin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 30 07:51:48 PDT 2022


danilaml added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:1308
+    if (SI->getDefaultDest() == PN.getParent())
+      return nullptr;
     Cond = SI->getCondition();
----------------
nikic wrote:
> I don't think this is sufficient. Can you please add a test where the default doesn't directly go to the phi block, but have one dummy block in between? (That is, go to a sw.19 block from both 19 and default.)
> 
> I think the right fix would be adding a `++SuccCount[SI->getDefaultDest()]`.
I'll add this test case, but I'm pretty sure that this is covered by the dominance check
```
DT.dominates(BasicBlockEdge(IDom, It->second),
                          BasicBlockEdge(Pred, BB));
```
I think this check will only succeed when both edges are the "same".

Then again, I think your proposed fix would work as well (at the expense of slightly more work), so I can switch to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126650



More information about the llvm-commits mailing list