riscv-non-isa / riscv-c-api-doc

Documentation of the RISC-V C API
https://jira.riscv.org/browse/RVG-4
Creative Commons Attribution 4.0 International
68 stars 38 forks source link

Adding target attribute for function. #35

Closed kito-cheng closed 10 months ago

kito-cheng commented 1 year ago

After adding mapping symbol and .option arch, finally we have all necessary components of function multi-versioning.

This proposal has add a new funciton attribte: target attribte for compile a funciton with specify extension, pipeline model or cost model, could be used as simple* function multi-versioning.

*: Simple is compare to target_clones in x86 and target_version in AArch64, and also require user to dispatch that right.

It similar to AArch64's target attribte: https://gcc.gnu.org/onlinedocs/gcc/AArch64-Function-Attributes.html

This attribte won't change the function name mangling, and also require do not update Tag_RISCV_arch attribte.

Ref:


NOTE: This PR will not be merged until we have PoC AND GNU/LLVM community reach consensus NOTE: We don't have PoC for either GCC and clang yet,

kito-cheng commented 1 year ago

cc @asb @luismarques @maskray @jrtc27 @topperc @preames @palmer-dabbelt @vineetgarc

kito-cheng commented 1 year ago

PoC for LLVM: https://reviews.llvm.org/D151730

palmer-dabbelt commented 1 year ago

Is this meant to plumb into function multi versioning as well, or just control single-versioned functions? Either way Jeff might have something to say, not sure what his github ID is though...

kito-cheng commented 1 year ago

This is for single-versioned functions or you can called manually multi-versioning since it must be gated by user, and multi-versioning will based on this and then extend, might be target_clone and/or target_version, e.g. target_clone ("same-syntax-as-target-attr1", "same-syntax-as-target-attr2")

kito-cheng commented 1 year ago

Either way Jeff might have something to say, not sure what his github ID is though...

Should be this one @JeffreyALaw

kito-cheng commented 1 year ago

Changes:

kito-cheng commented 1 year ago

Changes:

TODO:

kito-cheng commented 11 months ago

Changes:

kito-cheng commented 10 months ago

Minor tweaks

kito-cheng commented 10 months ago

cc @maskray, who might interested on this topic

cmuellner commented 10 months ago

I've reached out to the sig-toolchains list with a request for review with the intend to merge this PR in two weeks.

The planned timeline is as follows:

Oct 13: Send request for review to sig-toolchains list Oct 27: End of review period (PR will be merged if there are no outstanding issues)

preames commented 10 months ago

To provide a bit more context on the in person conversation last week...

@kito-cheng and I sat down at the LLVM developers meeting to go over the then current draft text and identified a couple of last minor issues. I provided a verbal LGTM at that point, subject to the changes identified being made. @topperc and @maskray joined in for the last part of our spoken conversation just before I ran off to a talk. I'm unclear if all their concerns were resolved. I think they were, but I don't want to speak for them.

One point I want to call out in our discussion is that we tentatively decided not to implement function multi-version semantics for target in C++. (On X86, C and C++ semantics for this attribute differ with C++ providing a form of a overloading based on attribute.) We're going to leave that case to target_clone and keep the plain target attribute simpler. This does result in a difference with other architectures, but given aarch64 and x64 already seem to differ significantly on the semantics assigned to this attribute, that seemed acceptable.

kito-cheng commented 10 months ago

Minor revise:

kito-cheng commented 10 months ago

Reach consensus on error when multiple __attribute__((target)) and duplicated value (same type of attribute) at RISC-V LLVM sync meeting.

Action item: Kito will discuss in next RISC-V GCC sync meeting, make sure everyone is happy with that.

MaskRay commented 10 months ago

For x86, GCC in C++ mode (but not in C mode) enables function multi-versioning for the following setup:

__attribute__((target("default"))) int foo(void) { return 0; }
__attribute__((target("sse4.2"))) int foo(void) { return 1; }

I think this is confusing. For RISC-V I think it makes sense not to enable function multi-versioning or make __attribute__((target("sse4.2"))) part of the mangling.

kito-cheng commented 10 months ago

Changes:

kito-cheng commented 10 months ago

Also reach consensus at RISC-V GNU community :)

@maskray let me know if the spec is look good to you.

preames commented 10 months ago

This has been under active discussion for months, and recently went through the 2 week comment period without significant comment. I think it's time to move forward here.

kito-cheng commented 10 months ago

Let moving forward, thanks everyone :)