Page MenuHomePhabricator

[NVPTX] Auto-upgrade some NVPTX intrinsics to LLVM target-generic code.

Authored by jlebar on Jan 16 2017, 11:17 PM.



Specifically, we upgrade llvm.nvvm.:

  • brev{32,64}
  • clz.{i,ll}
  • popc.{i,ll}
  • abs.{i,ll}
  • {min,max}.{i,ll,u,ull}
  • h2f

These either map directly to an existing LLVM target-generic
intrinsic or map to a simple LLVM target-generic idiom.

In all cases, we check that the code we generate is lowered to PTX as we

This patch also adds implementations of the corresponding builtins to

Diff Detail


Event Timeline

jlebar created this revision.Jan 16 2017, 11:17 PM
tra added inline comments.Jan 17 2017, 2:16 PM
124 ↗(On Diff #84633)

Shouldn't that be _ll ? That was the name of the max of long long arguments in BuiltinsNVPTX.def.
Speaking of which, it would need to have builtins removed, too.

127 ↗(On Diff #84633)


135 ↗(On Diff #84633)


138 ↗(On Diff #84633)


142 ↗(On Diff #84633)

Do we still need int_nvvm_h2f in ?

How about their int_nvvm_f2h* counterparts?

jlebar updated this revision to Diff 85076.Jan 19 2017, 5:40 PM

Remove (broken) implementations of removed intrinsics from clang.

We realized we don't need this, as none of these intrinsics are accessible to
user code in nvcc.

jlebar marked 4 inline comments as done.Jan 19 2017, 5:40 PM

Sorry about that broken code in the clang headers. All removed now; we established it's not needed. PHAL.

tra edited edge metadata.Jan 20 2017, 10:01 AM

Now that we've removed ton of @llvm.nvvm intrinsics from .td files, we have no easy way to tell that we still do support these intrinsics (mostly for the sake of libdevice?) by upgrading them.
Perhaps we should add the list of such intrinsics and what happens to them in a comment section in

207 ↗(On Diff #85076)

There are still remnants of __nvvm_{brev,abs,clz,...} in include/clang/Basic/BuiltinsNVPTX.def
Now that llvm does not provide them, they should be removed from clang as well, IMO.

jlebar updated this revision to Diff 85189.Jan 20 2017, 1:58 PM
jlebar marked an inline comment as done.

Address tra's review comments.

Perhaps we should add the list of such intrinsics and what happens to them in a comment section in

sgtm, done.

tra accepted this revision.Jan 20 2017, 2:11 PM

There are couple of bits to be deleted. LGTM otherwise.

671–672 ↗(On Diff #85189)

This one should be gone, too.

770–771 ↗(On Diff #85189)

Same here.

This revision is now accepted and ready to land.Jan 20 2017, 2:11 PM
jlebar updated this revision to Diff 85206.Jan 20 2017, 3:23 PM

Get rid of h2f intrinsics.

Thank you for the careful reviews, Art.

This revision was automatically updated to reflect the committed changes.