This is an archive of the discontinued LLVM Phabricator instance.

[PGO] Avoid assertion on profile generated from code without an exit block
ClosedPublic

Authored by wolfgangp on Sep 19 2022, 11:40 AM.

Details

Summary

A degenerate profile can cause an assertion at code gen time. Specifically, we have a non-zero basic block count with 0 counts in all of the successor blocks.

This has occurred in practice at least once, and while the circumstances under which the profile was created were not immediately reproducible, the crash can be avoided by guarding the offending call.

Diff Detail

Unit TestsFailed

Event Timeline

wolfgangp created this revision.Sep 19 2022, 11:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2022, 11:40 AM
wolfgangp requested review of this revision.Sep 19 2022, 11:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2022, 11:40 AM

This looks like related to infinite loops.

xur added a comment.EditedSep 19 2022, 2:25 PM

This looks like related to infinite loops.

Just found there is a test case in this patch. Let me take a look at why this is happening.

xur added a comment.Sep 19 2022, 2:28 PM

I'm fine with change. But can you add a warning message here? I would be helpful to know something is wrong here.

xur added a comment.Sep 19 2022, 3:26 PM

This is an interesting example. When we design this, we assume there is at least one exit node in the function. And we create fake edges from exit node(s). This way we create a close graph.

For this case, we don't have an exit node and I think the function exits from a non-return call to "zeroexit". So the edge propagation will terminate incompletely.

I think this is a corner case but let me think how to handle this.

xur added a comment.EditedSep 21 2022, 11:31 AM

This is an interesting example. When we design this, we assume there is at least one exit node in the function. And we create fake edges from exit node(s). This way we create a close graph.

For this case, we don't have an exit node and I think the function exits from a non-return call to "zeroexit". So the edge propagation will terminate incompletely.

I think this is a corner case but let me think how to handle this.

I read the code again and I think a closed graph is not required for the propagation. The flow will be inconsistent but there is little we can do about it.
The root cause is we don't break function call into a separated BB. So the implicit edges from non-return calls are not accountable.

We probably don't want to change this because it will slow down the instrumentation and the benefit is not minimal for PGO performance.

I think this fix is fine.

But there are a few minor things need to change
(1) the title: the problem is not from degenerate profile. I can create a prefect legal profile and reproduce the assertion
(2) add a waring or debug trace

You also probably want to use a reduce test case.
Here is the test case I created:

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define void @bar(i32 noundef %s) {
entry:
  %cmp = icmp sgt i32 %s, 20
  br i1 %cmp, label %if.then, label %if.end

if.then:
  call void @exit(i32 noundef 1)
  unreachable

if.end:
  ret void
}

declare void @exit(i32 noundef)

define void @foo(i32 noundef %n) {
entry:
  %sum = alloca i32, align 4
  store volatile i32 %n, ptr %sum, align 4
  %sum.0.sum.0. = load volatile i32, ptr %sum, align 4
  call void @bar(i32 noundef %sum.0.sum.0.)
  %cmp = icmp slt i32 %n, 10
  br i1 %cmp, label %if.then, label %if.end

if.then:
  %sum.0.sum.0.1 = load volatile i32, ptr %sum, align 4
  call void @bar(i32 noundef %sum.0.sum.0.1)
  br label %if.end

if.end:
  br label %for.cond

for.cond:
  %sum.0.sum.0.2 = load volatile i32, ptr %sum, align 4
  call void @bar(i32 noundef %sum.0.sum.0.2)
  br label %for.cond
}

profile file:

# IR level Instrumentation Flag
:ir
foo
# Func Hash:
146835645050580305
# Num Counters:
3
# Counter Values:
0
1
0

Sorry about the delay. Thank you for your test case, I have used it instead of the original one and added a warning.
Please check if the wording describes the issue reasonably correctly and succinctly, as the circumstances are somewhat arcane.

wolfgangp retitled this revision from [PGO] Avoid assertion on degenerate profile to [PGO] Avoid assertion on profile generated from code without an exit block.Sep 30 2022, 11:08 AM
xur accepted this revision.Oct 12 2022, 10:35 AM

Looks good to me.

This revision is now accepted and ready to land.Oct 12 2022, 10:35 AM
This revision was landed with ongoing or failed builds.Oct 13 2022, 2:58 PM
This revision was automatically updated to reflect the committed changes.