Page MenuHomePhabricator

AMDGPU: Remove gcc builtin names from workitem intrinsics
ClosedPublic

Authored by jvesely on May 26 2016, 10:48 AM.

Details

Summary

We'll need to emit these manually in clang to add range metadata.

Names in llvm/IR/Intrinsics*.td are considered before custom emit so they need to go.

Diff Detail

Repository
rL LLVM

Event Timeline

jvesely updated this revision to Diff 58652.May 26 2016, 10:48 AM
jvesely retitled this revision from to AMDGPU: Remove gcc builtin names from workitem intrinsics.
jvesely updated this object.
jvesely added reviewers: tstellarAMD, arsenm.
jvesely set the repository for this revision to rL LLVM.
jvesely added a subscriber: llvm-commits.
arsenm accepted this revision.May 26 2016, 11:13 AM
arsenm edited edge metadata.

Why do you need this? I know I had a patch to do the same thing at some point, but don't remember why. Since these don't need mangling I would expect GCCBuiltin to work correctly

This revision is now accepted and ready to land.May 26 2016, 11:13 AM

Why do you need this? I know I had a patch to do the same thing at some point, but don't remember why. Since these don't need mangling I would expect GCCBuiltin to work correctly

clang in CodeGenFunction::EmitBuiltinExpr() considers embedded intrinsic names before calling EmitTargetBuiltinExpr(). Since we want these intrinsics to be emitted with range metadata we need to do it in EmitAMDGPUBuiltinExpr(called by EmitTargetBUiltinExpr). The embedded names need to be removed, otherwise the Emit* functions won't run.

jvesely updated this revision to Diff 58672.May 26 2016, 12:55 PM
jvesely edited edge metadata.

global offset is unbounded so we can provide the GCC builtin here

jvesely updated this revision to Diff 58687.May 26 2016, 1:38 PM

ngroups, global size, and group id are also unbounded.

sorry for the noise

jvesely requested a review of this revision.May 26 2016, 1:38 PM
jvesely edited edge metadata.
arsenm accepted this revision.May 26 2016, 2:00 PM
arsenm edited edge metadata.

LGTM

This revision is now accepted and ready to land.May 26 2016, 2:00 PM
jvesely updated this revision to Diff 61431.Jun 21 2016, 1:00 PM
jvesely edited edge metadata.

rebase to remove dependence on D20298

LGTM

include/llvm/IR/IntrinsicsAMDGPU.td
41 ↗(On Diff #61431)

This comment isn't needed / is misleading since the intrinsic itself has no indication of the range limit

jvesely marked an inline comment as done.Jun 21 2016, 1:39 PM
jvesely added inline comments.
include/llvm/IR/IntrinsicsAMDGPU.td
41 ↗(On Diff #61431)

Removed locally. IMO it would be preferable if the range metadata were tied to the intrinsic rather than added by clang.

This revision was automatically updated to reflect the committed changes.
jvesely marked an inline comment as done.