Page MenuHomePhabricator

[COFF, ARM64] Add MS intrinsics: __getReg, _ReadStatusReg, _WriteStatusReg
AbandonedPublic

Authored by mgrang on Aug 23 2018, 6:11 PM.

Details

Summary

Added declarations for the intrinsics in intrin.h. These are defined in MSVC libs and are needed for
certain spec2000 benchmarks.

Diff Detail

Event Timeline

mgrang created this revision.Aug 23 2018, 6:11 PM

In MSVC, these ones seem to be declared in intrin.h, not in the arm64intr.h subheader, so to match closely perhaps we should declare them there as well?

The test, arm64-microsoft-intrinsics.c, doesn't actually include intrin.h nor arm64intr.h, so this test doesn't actually test anything of what this changes (this test would pass just as well before this change). Not sure if a test for this is mandatory or how we handle tests for lib/Headers though.

mgrang updated this revision to Diff 162431.Aug 24 2018, 11:24 AM
mgrang edited the summary of this revision. (Show Details)

Thanks @mstorsjo. I have addressed your comments.

rnk added a comment.Aug 24 2018, 11:40 AM

This just adds declarations. Are these functions provided by some Windows runtime library? Don't you need to implement the intrinsics somewhere in order for the apps that use them to run or link? Ideally, they should be implemented as builtins.

mgrang abandoned this revision.Oct 2 2018, 4:00 PM

Will abandon this patch since I have implementations of these which I will upstream soon.

dmajor added a subscriber: dmajor.Oct 22 2018, 3:37 PM

Will abandon this patch since I have implementations of these which I will upstream soon.

Just to link up the reviews: these landed in D52838 and D53115. (Thanks @mgrang!)

Will abandon this patch since I have implementations of these which I will upstream soon.

Just to link up the reviews: these landed in D52838 and D53115. (Thanks @mgrang!)

Except not really... D53115 got the types wrong, and used int instead of __int64.