This is an archive of the discontinued LLVM Phabricator instance.

[WIP][ValueTracking] Make use of CtxI info in programUndefinedIfUndefOrPoison
Changes PlannedPublic

Authored by StephenFan on May 14 2023, 12:10 AM.

Details

Summary

There is also two test file crashed. The motivation to do this is I
found that the following file was miscompiled:

define void @zext_sdiv_not_ok3_maybe_poison_denum(i32 noundef %nn, i16 %xx) {
entry:
  %n = and i32 %nn, 123
  %x = or i16 %xx, 1
  br label %loop
loop:
  call void @maythrow()
  %zext = zext i16 %x to i32
  %div = sdiv i32 %n, %zext
  call void @use.i32(i32 %div)
  br label %loop
}

LICM hoisted sdiv to entry block incorrectly. Since if %xx is poison,
sdiv is UB.

Diff Detail

Event Timeline

StephenFan created this revision.May 14 2023, 12:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 14 2023, 12:10 AM
StephenFan requested review of this revision.May 14 2023, 12:10 AM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
nikic added a comment.May 14 2023, 5:46 AM

I don't understand the example from the patch description. At the point sdiv is hoisted, the zext will be in the preheader and as such should not satisfy programUndefinedIfUndefOrPoison().

Generally, the way this function is implemented requires the scan to start at the instruction. Simplify starting from a different place doesn't really make sense, because the YieldsPoison set will not be populated correctly.

I don't understand the example from the patch description. At the point sdiv is hoisted, the zext will be in the preheader and as such should not satisfy programUndefinedIfUndefOrPoison().

Generally, the way this function is implemented requires the scan to start at the instruction. Simplify starting from a different place doesn't really make sense, because the YieldsPoison set will not be populated correctly.

You are right. I reimplemented a patch in D150542. I hope the description of that patch is clear. And I think we still can optimize programUndefinedIfUndefOrPoison to make use of CtxI.

StephenFan planned changes to this revision.May 15 2023, 12:36 AM