Page MenuHomePhabricator

[IfConversion] Fix diamond conversion with unanalyzable branches.

Authored by efriedma on Sep 4 2019, 5:48 PM.



The code was incorrectly counting the number of identical instructions, and therefore tried to predicate an instruction which should not have been predicated. This could have various effects: a compiler crash, an assembler failure, a miscompile, or just generating an extra, unnecessary instruction.

Instead of depending on TargetInstrInfo::removeBranch, which only works on analyzable branches, just remove all branch instructions.

Fixes and .

Diff Detail


Event Timeline

efriedma created this revision.Sep 4 2019, 5:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 4 2019, 5:48 PM
Herald added a subscriber: javed.absar. · View Herald Transcript
bjope added a subscriber: bjope.Sep 4 2019, 10:03 PM
dmgreen accepted this revision.Sep 5 2019, 2:49 AM


This revision is now accepted and ready to land.Sep 5 2019, 2:49 AM
This revision was automatically updated to reflect the committed changes.
hans added a subscriber: hans.Sep 6 2019, 1:05 AM
hans added inline comments.

The previous code also decremented BBI1->NonPredSize. Do we not need to do that anymore?

(I don't know this code, just driving by.)

efriedma marked an inline comment as done.Sep 6 2019, 12:44 PM
efriedma added inline comments.

In theory, we probably should decrement NonPredSize, for both this loop and the following loop, to account for the instructions we're erasing.

We only use the NonPredSize as input to the if-conversion heuristics, though. It's unlikely we're going to predicate a basic block that's already predicated. So whatever we do probably has very little, if any, effect.