[PATCH] D105363: [InstCombine] Transitively propagate `unreachable` into predecessors

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D105363#2865190 <https://reviews.llvm.org/D105363#2865190>, @lebedev.ri wrote:

> In D105363#2865153 <https://reviews.llvm.org/D105363#2865153>, @spatel wrote:
>
>> In D105363#2863232 <https://reviews.llvm.org/D105363#2863232>, @lebedev.ri wrote:
>>
>>> @spatel thoughts?
>>
>> I haven't closely followed the related patches, so I'm probably not adding much here...
>> But if I run -simplifycfg on any of the tests with diffs here, they are completely zapped down to a single `unreachable`. 
>> Can you add a test to demonstrate where we might fall through the cracks of both passes? (possibly a test for PhaseOrdering)
>
> I'm not really sure if there is any pattern that fits your description.
> The goal/question here is mostly: should we, the instcombine, be able to get rid of more known-dead code,
> potentially allowing further folds, without deferring until the next time instcombine runs after simplifycfg runs after us?

I'm leaning towards "no" then. It seems similar to me to arguments between CSE and InstCombine. For example, we could try harder to make InstCombine see through identical values via pattern matching, but we intentionally do not try that because we have passes dedicated to that functionality. Here, we have a pass (SimplifyCFG) that can and does have the logic to propagate unreachable code, so I don't think we should duplicate that effort for a potential small improvement in efficiency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105363