Page MenuHomePhabricator

[AArch64] [ARM] Make a target-independent llvm.thread.pointer intrinsic.
ClosedPublic

Authored by koriakin on Apr 14 2016, 1:06 AM.

Details

Summary

Both AArch64 and ARM support llvm.<arch>.thread.pointer intrinsics that
just return the thread pointer. I have a pending patch that does the same
for SystemZ (D19054), and there are many more targets that could benefit
from one.

This patch merges the ARM and AArch64 intrinsics into a single target
independent one that will also be used by subsequent targets.

This breaks a clang test (that checks for llvm.aarch64.thread.pointer),
which will be fixed in another change. clang itself requires no modification,
thanks to GCCBuiltin.

Diff Detail

Repository
rL LLVM

Event Timeline

koriakin retitled this revision from to [AArch64] [ARM] Make a target-independent llvm.thread.pointer intrinsic..
koriakin updated this object.
koriakin added a reviewer: t.p.northover.
koriakin set the repository for this revision to rL LLVM.
koriakin added a subscriber: llvm-commits.
ab added a subscriber: ab.Apr 14 2016, 8:39 AM

I think you also need to auto-upgrade (lib/IR/AutoUpgrade.cpp) the old intrinsics to the new one.

koriakin planned changes to this revision.Apr 14 2016, 11:36 AM
In D19098#401268, @ab wrote:

I think you also need to auto-upgrade (lib/IR/AutoUpgrade.cpp) the old intrinsics to the new one.

Done.

ab added a comment.Apr 18 2016, 10:57 AM

Seems reasonable to me, but I think it'd make more sense to test the autoupgrade separately. What about an llvm-as|llvm-dis test in test/Assembler?

In D19098#404133, @ab wrote:

Seems reasonable to me, but I think it'd make more sense to test the autoupgrade separately. What about an llvm-as|llvm-dis test in test/Assembler?

Done.

ab accepted this revision.Apr 18 2016, 12:19 PM
ab added a reviewer: ab.

LGTM with a couple extra nits, thanks!

test/Assembler/thread-pointer.ll
1 ↗(On Diff #54091)

Rename the file to something like autoupgrade-thread-pointer.ll ?

10 ↗(On Diff #54091)

-tail

test/CodeGen/AArch64/arm64-builtins-linux.ll
14–19 ↗(On Diff #54091)

I think you can remove the codegen tests with the old intrinsics now.

This revision is now accepted and ready to land.Apr 18 2016, 12:19 PM
koriakin edited edge metadata.
koriakin marked 2 inline comments as done.

Thanks, now waiting for acceptance of the corresponding clang patch (D19099).

Should this intrinsic get documented in the langref?

Should this intrinsic get documented in the langref?

Sounds like a good plan, I'll throw it in to Code Generator Intrinsics.

Now includes documentation in langref.

ab added a comment.Apr 19 2016, 9:38 AM

Thanks David & Marcin! Langref LGTM as well.

docs/LangRef.rst
9620 ↗(On Diff #54096)
i8* @llvm...
This revision was automatically updated to reflect the committed changes.