Page MenuHomePhabricator

[amdgpu] Add `llvm.amdgcn.endpgm` support.
ClosedPublic

Authored by hliao on Wed, Nov 4, 6:24 PM.

Details

Summary
  • llvm.amdgcn.endpgm is added to enable "abort" support.

Diff Detail

Event Timeline

hliao created this revision.Wed, Nov 4, 6:24 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptWed, Nov 4, 6:24 PM
hliao requested review of this revision.Wed, Nov 4, 6:24 PM
rampitec added inline comments.Thu, Nov 5, 10:18 AM
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
1581

Mayby also IntrCold?

hliao updated this revision to Diff 303213.Thu, Nov 5, 11:51 AM

Add IntrCold.

hliao marked an inline comment as done.Thu, Nov 5, 11:51 AM
hliao added inline comments.
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
1581

Good point!

Should this also be IntrConvergent?

Should this also be IntrConvergent?

Probably yes... This is control flow after all.

hliao marked an inline comment as done.Thu, Nov 5, 1:13 PM

Should this also be IntrConvergent?

Probably yes... This is control flow after all.

The real effect of this intrinsic seems not changed if it's moved somewhere. For example,

convert this

if (a || b)
  endpgm();

to

if (a)
  endpgm();
else if (b)
  endpgm();

no matter a or b are divergent or not, it works the same as the original one as s_endpgm is a scalar instruction. Ofc, if we really doubt bad things may happen, I could add that for safety as we definitely will revise this later.

rampitec accepted this revision.Thu, Nov 5, 2:08 PM

Should this also be IntrConvergent?

Probably yes... This is control flow after all.

The real effect of this intrinsic seems not changed if it's moved somewhere. For example,

convert this

if (a || b)
  endpgm();

to

if (a)
  endpgm();
else if (b)
  endpgm();

no matter a or b are divergent or not, it works the same as the original one as s_endpgm is a scalar instruction. Ofc, if we really doubt bad things may happen, I could add that for safety as we definitely will revise this later.

It is probably OK to clone it, we shall insert skips around it anyway. LGTM.

This revision is now accepted and ready to land.Thu, Nov 5, 2:08 PM
This revision was landed with ongoing or failed builds.Thu, Nov 5, 4:07 PM
This revision was automatically updated to reflect the committed changes.
foad added a subscriber: foad.Fri, Nov 6, 1:05 AM
foad added inline comments.
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
1580

The intrinsic def needs a comment. Is it supposed to literally just generate an s_endpgm instruction, or is it something more high-level?