SimpleLoopUnswitch incorrectly leaves make.implicit MD when there's an infinite loop

[Quuxplusone]

Bugzilla Link	PR47513
Status	NEW
Importance	P enhancement
Reported by	Daniil Suchkov (suc-daniil@yandex.ru)
Reported on	2020-09-13 23:10:06 -0700
Last modified on	2020-09-15 21:04:25 -0700
Version	trunk
Hardware	PC Linux
CC	efriedma@quicinc.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Here's a test case:
cmd: opt -enable-nontrivial-unswitch=true -
passes='loop(unswitch),verify<loops>' -S < %s

define i32 @test_should_drop_make_implicit.infinite_loop(i32* %p1, i32* %p2,
i1* %p3) {
entry:
  %null_check = icmp eq i32* %p2, null
  br label %loop.header
loop.header:
  %iv = phi i32 [0, %entry], [%iv.next, %backedge]
  br label %possibly_infinite_loop
possibly_infinite_loop:
  %inner_loop.cond = load i1, i1* %p3
  br i1 %inner_loop.cond, label %possibly_infinite_loop, label %loop.body
loop.body:
  %x = load i32, i32* %p1
  %side_exit_cond = icmp eq i32 %x, 0
  br i1 %null_check, label %throw_npe, label %backedge, !make.implicit !0
backedge:
  %iv.next = add i32 %iv,1
  %loop_cond = icmp slt i32 %iv.next, 10000
  br i1 %loop_cond, label %loop.header, label %exit
throw_npe:
  call void @throw_npe()
  unreachable
exit:
  ret i32 %x
}

Due to a possibly infinite loop between the outer loop entry and the null
check, make.implicit MD should be dropped when we unswitch the loop on
%null_check condition. However, currently we keep that MD.

[Quuxplusone]

While you're looking at this, could you clarify the definition of make.implicit? The definition of make.implicit in http://llvm.org/docs/FaultMaps.html describes the expectation for frontends, but not for transforms.

[Quuxplusone]

The framed problem here is that, under condition of loop's infinity, we may never recover if the pointer is in fact null (recovery in this example is supposed to happen in throw_npe()). However this kind of bugs (non-revocery) is not a correctness issue. It can only become a performance issue if we step on the null over and over again and keep walking through signal handling mechanism, which is slow. But if we are stuck in the infinite loop, it's not going to happen.

So I don't think it's an issue at all.