This is an archive of the discontinued LLVM Phabricator instance.

Teach ValueTracking that compare-and-branch on poison is UB

Authored by sanjoy on Apr 17 2016, 8:51 PM.



This lets us optimize the interesting case of

do {
  I = PHI(I_Begin, I_Next);
  I_Next = I +nsw 1
} while (I_Next < N);

Note: we "already get" the above case, but the way we do so is
incorrect. Making our poison value analysis smarter about this will let
us get rid of the buggy behavior (in a later patch) without regressing
too much performance across the board.

Diff Detail

Event Timeline

sanjoy updated this revision to Diff 54028.Apr 17 2016, 8:51 PM
sanjoy retitled this revision from to Teach ValueTracking that compare-and-branch on poison is UB.
sanjoy updated this object.
sanjoy added reviewers: broune, bjarke.roune, majnemer.
sanjoy added a subscriber: llvm-commits.
majnemer added inline comments.Apr 17 2016, 11:43 PM

Our langref states:

An instruction control-depends on a terminator instruction if the terminator instruction has multiple successors and the instruction is always executed when control transfers to one of the successors, and may not be executed when control is transferred to another.

The code, as written, sounds like it will do the wrong thing if the branch instruction branches to the same basic block regardless of the condition operand. Maybe this is just a bug in the langref? Equally possible is that I misunderstand the behavior that this function is trying to suss out.

sanjoy added inline comments.Apr 18 2016, 12:07 AM

You're right; as written this code is not correct and it can't easily be fixed either. Going by the langref, this code fragment (the motivation for this change)

  %iv = phi(0, = add nsw %iv, 1
  %cmp = s< %L
  br i1 %cmp, label %leave, label %loop

is well defined, even when is poison.


sanjoy abandoned this revision.Apr 20 2016, 10:55 PM

As @majnemer pointed out, this change is wrong.