This is an archive of the discontinued LLVM Phabricator instance.

NVPTX: Lower unreachable to exit to allow ptxas to accurately reconstruct the CFG.
ClosedPublic

Authored by maleadt on Jun 13 2023, 2:06 AM.

Details

Summary

PTX does not have a notion of unreachable, which results in emitted basic
blocks having an edge to the next block:

block1:
  call @does_not_return();
  // unreachable
block2:
  // ptxas will create a CFG edge from block1 to block2

This may result in significant changes to the control flow graph, e.g., when
LLVM moves unreachable blocks to the end of the function. That's a problem
in the context of divergent control flow, as ptxas uses the CFG to determine
divergent regions, while some intructions may not be executed divergently.

For example, bar.sync is not allowed to be executed divergently on Pascal
or earlier. If we start with the following:

entry:
  // start of divergent region
  @%p0 bra cont;
  @%p1 bra unlikely;
  ...
  bra.uni cont;
unlikely:
  ...
  // unreachable
cont:
  // end of divergent region
  bar.sync 0;
  bra.uni exit;
exit:
  ret;

it is transformed by the branch-folder and block-placement passes to:

entry:
  // start of divergent region
  @%p0 bra cont;
  @%p1 bra unlikely;
  ...
  bra.uni cont;
cont:
  bar.sync 0;
  bra.uni exit;
unlikely:
  ...
  // unreachable
exit:
  // end of divergent region
  ret;

After moving the unlikely block to the end of the function, it has an edge
to the exit block, which widens the divergent region and makes the bar.sync
instruction happen divergently. That causes wrong computations, as we've been
running into for years with Julia code (which emits a lot of trap +
unreachable code all over the place).

To work around this, add an exit instruction before every unreachable,
as ptxas understands that exit terminates the CFG. Note that trap is not
equivalent, and only future versions of ptxas will model it like exit.
Another alternative would be to emit a branch to the block itself, but emitting
exit seems like a cleaner solution to represent unreachable to me.

Also note that this may not be sufficient, as it's possible that the block
with unreachable control flow is branched to from different divergent regions,
e.g. after block merging, in which case it may still be the case that ptxas
could reconstruct a CFG where divergent regions are merged (I haven't confirmed
this, but also haven't encountered this pattern in the wild yet):

entry:
  // start of divergent region 1
  @%p0 bra cont1;
  @%p1 bra unlikely;
  bra.uni cont1;
cont1:
  // intended end of divergent region 1
  bar.sync 0;
  // start of divergent region 2
  @%p2 bra cont2;
  @%p3 bra unlikely;
  bra.uni cont2;
cont2:
  // intended end of divergent region 2
  bra.uni exit;
unlikely:
  ...
  exit;
exit:
  // possible end of merged divergent region?

I originally tried to avoid the above by cloning paths towards unreachable and
splitting the outgoing edges, but that quickly became too complicated. I propose
we go with the simple solution first, also because modern GPUs with more flexible
hardware thread schedulers don't even suffer from this issue.

Finally, although I expect this to fix most of
https://bugs.llvm.org/show_bug.cgi?id=27738, I do still encounter
miscompilations with Julia's unreachable-heavy code when targeting these
older GPUs using an older ptxas version (specifically, from CUDA 11.4 or
below). This is likely due to related bugs in ptxas which have been fixed
since, as I have filed several reproducers with NVIDIA over the past couple of
years. I'm not inclined to look into fixing those issues over here, and will
instead be recommending our users to upgrade CUDA to 11.5+ when using these GPUs.

Also see:

Diff Detail

Event Timeline

maleadt created this revision.Jun 13 2023, 2:06 AM
maleadt requested review of this revision.Jun 13 2023, 2:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2023, 2:06 AM
vchuravy added a project: Restricted Project.Jun 13 2023, 10:11 AM
vchuravy added a subscriber: vchuravy.
tra accepted this revision.Jun 13 2023, 10:46 AM

It's unfortunate that we don't seem to have a way to lower unreachable via normal lowering mechanisms. AFAICT we don't even have a selection DAG node type for it.

This revision is now accepted and ready to land.Jun 13 2023, 10:46 AM

Why do we need the structurizer pass as well?

maleadt updated this revision to Diff 532107.Jun 16 2023, 5:22 AM

Don't use the structurizer pass.

Why do we need the structurizer pass as well?

We don't, I accidentally added that to this PR.

I removed it, but I'm not sure if I did the right arc incantation (arc diff --update D152789) to push that change, as it's the only one the UI now shows.

maleadt updated this revision to Diff 532157.Jun 16 2023, 8:10 AM

Push the entire diff again.

jdoerfert accepted this revision.Jun 16 2023, 9:56 AM

LG, thanks

I don't have commit access; can somebody land this for me? Or does this require more review?

tra added a comment.Jun 20 2023, 9:41 AM

I don't have commit access; can somebody land this for me? Or does this require more review?

I can land it. How do you want me to attribute the patch? Should it be Tim Besard <tim@juliacomputing.com> or something else?

Tim Besard <tim@juliahub.com> would be better, thanks.

This revision was landed with ongoing or failed builds.Jun 21 2023, 11:41 AM
This revision was automatically updated to reflect the committed changes.