Page MenuHomePhabricator

[if-converter] Handle BB that terminate in ret during diamond conversion
AbandonedPublic

Authored by vchuravy on Apr 3 2018, 11:27 AM.

Details

Reviewers
kparzysz
iteratee
Summary

While I was updating the Julia frontend for LLVM 6.0.0 I encountered a situation
where we emitted code that triggered the diamond conversion in if-converter while
TrueBB and FalseBB terminated in ret.

      BB
    /    \
TrueBB  FalseBB
  ...     ...
  ret     ret

The if-converter pass decided that this was a valid structure to do a diamond conversion on
and this triggered an assertion later on since if-converter implictly assumed that TrueBB and
FalseBB terminate in a branch or a fall-trough. Since the situation described above is equivalent
to:

      BB
    /    \
TrueBB  FalseBB
  ...     ...
    \     /
       BB
       ret

I think it is reasonable for if-converter to do a diamond conversion on and so this PR adds a bit handling for
return (treating returns as a form of an unconditional branch).

This fixes https://bugs.llvm.org/show_bug.cgi?id=36825

Diff Detail

Event Timeline

vchuravy created this revision.Apr 3 2018, 11:27 AM
vchuravy edited the summary of this revision. (Show Details)Apr 3 2018, 11:28 AM
vchuravy updated this revision to Diff 140901.Apr 3 2018, 9:30 PM

@nemanjai correctly noted that the both branches are equal and we can simplify this,
by making sure that we don't fall over the iterator and then removing both BB.

My previous version causes problems for other test cases and this version passes my minimal test
and the case found in the Julia frontend.

I encountered a related problem in Hexagon code and I amended the patch to handle that as well. I couldn't upload it to this review, so I created a new one: D45819, but it's really meant to be an extension of this fix.