This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Support overloaded version ntlh intrinsic function
ClosedPublic

Authored by BeMg on Jul 25 2023, 2:49 AM.

Details

Summary

Here is the proposal https://github.com/riscv-non-isa/riscv-c-api-doc/pull/47.

The version that omit the domain argument imply domain=__RISCV_NTLH_ALL.

type __riscv_ntl_load (type *ptr);
void __riscv_ntl_store (type *ptr, type val);

Diff Detail

Event Timeline

BeMg created this revision.Jul 25 2023, 2:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 25 2023, 2:49 AM
BeMg requested review of this revision.Jul 25 2023, 2:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 25 2023, 2:49 AM
BeMg edited the summary of this revision. (Show Details)Jul 25 2023, 2:52 AM
BeMg added reviewers: kito-cheng, craig.topper, asb.
asb added a comment.Jul 25 2023, 5:05 AM

This seems functionally correct to me, but I'd welcome opinions from others who work more with the C intrinsics on if this is the best way to implement the overloading.

kito-cheng added inline comments.Jul 26 2023, 10:54 PM
clang/lib/Headers/riscv_ntlh.h
28

__SELECT_NTL_LOAD

39

__SELECT_NTL_STORE

BeMg updated this revision to Diff 544645.Jul 27 2023, 1:17 AM

Add prefix __riscv_ for all marco

BeMg marked 2 inline comments as done.Jul 28 2023, 1:27 AM

I think another option could be to do this in SemaChecking.cpp where we implement builtin_riscv_ntl_load and builtin_riscv_ntl_store. We already do custom type checking there. We could detect the missing argument and give it a default.

BeMg updated this revision to Diff 546324.Aug 1 2023, 11:03 PM

Use semacheck and CGBuiltin to support overload version functionY

kito-cheng accepted this revision.Aug 3 2023, 12:42 AM

LGTM, and the change seems like is fewer than my exception :)

This revision is now accepted and ready to land.Aug 3 2023, 12:42 AM
eopXD added inline comments.Aug 3 2023, 1:13 AM
clang/test/CodeGen/RISCV/ntlh-intrinsics/riscv32-zihintntl.c
218

Nit: new line here?

BeMg updated this revision to Diff 546753.Aug 3 2023, 1:54 AM

Add new line

This revision was landed with ongoing or failed builds.Aug 4 2023, 12:39 AM
This revision was automatically updated to reflect the committed changes.