-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
How to support backward incompatible API. #145
Comments
I think this might be the last chance to redesign RVV C intrinsic API as the v-spec is now ratified. So I guess we can have one last big remake before the API settles. I think 2. is better as it would unify with other extensions' intrinsic. Also I think it would be more intuitive if the new set of backward incompatible intrinsic are just named differently. We can allow the two headers to coexist and deprecate them in the future. |
The RVV GCC is in the RISC-V foundation repo but not push to GCC upstream yet. I don't think there is backward compatible issue in GCC because I can push them upstream after you freeze the document. Do you want me to support the old obsolete intrinsics before pushing them upstream? I am Ok with that. It I need to do so, Can you list the api both 0.9 version and 1.0 version together in the document? If the api are the same both 0.9 and 1.0 version. Just keep the document no need to change. |
I think we may not want to break backward compatibility in any sense. We may improve the user experience by extending the intrinsic API. Maybe we can discuss on how extensions can be added to the intrinsic after this v1.0 release? |
I want to raise the discussion regarding backward compatibility back again. I originally thought that maybe we should defer the problem of fixing these glitches to discussion after v1.0, however after a second thought, maybe v1.0 is the best timing to apply incompatible changes since we have not reached a "formal" release yet (nothing is ratified). I think that changes that is backed with functional or compiler optimization reason justifies themself. On the other hand, people may find some glitches in the current intrinsic API-s. For example, issue #159 pointed out that there is inconsistency on some functions where it can be expressed in reduce form without ambiguous naming but has output type appended in the function name. Another example is #140, where the argument orderings are inconsistent. Taking this further, we may always come back to the same dilemma, where we consider software stability on one side and improving the API on the other. So I think it is important we should take the courage to talk about this and rule out some SOP that we can follow. Assuming that an incompatible change have gathered approval from the community, how do we apply it? A straightforward approach is that the approved incompatible change be applied in the next release. A more conservative approach is we find a way to have deprecation warnings of the change in the next version, and have the change be applied in the next-next version. I personally think we should take a more conservative approach on this if we decide to formalize an approach to resolve modifications of incompatible change. Please share your thoughts on this. |
This issue like Vector ISA, maybe for all ISA version issue, +1 for separated specific version intrinsic header like as riscv_v_intrinsic_<version>.h |
We have several proposals which are trying to change the current API (ex. renaming or reorder the argument order), and some of them make the design more consistent and reasonable.
For examples:
These changes aren't backward compatible. If we want to make the design better and adapt these changes, we would face backward incompatibility. If we agree that we want to do this, then we need to have consensus on how to support that.
I have two proposals as shown below:
The Compiler could optionally support different versions of RVV API based on tags in
rvv-intrinsic-doc
. For now we havev0.10
andv0.9
tag. We can have a predefined macro__riscv_v_intrinsic_version
to imply the supported version information which follows the formula we already have in https://github.com/riscv-non-isa/riscv-c-api-doc/blob/master/riscv-c-api.md#architecture-extension-test-macro (<MAJOR_VERSION> * 1,000,000 + <MINOR_VERSION> * 1,000 + <REVISION_VERSION>
)We will have the opportunity to update API because we have now versioned them.
or
__rvv_
prefix in front of the intrinsics, and stored in a separate intrinsic header (with the header name following the rule proposed in Uniform intrinsic filename riscv-c-api-doc#17).For example, new naming will look like
vint8m1_t __rvv_vadd_vv_i8m1(vint8m1_t vs2, vint8m1_t vs1, size_t vl);
under header file name likeriscv_v_intrinsic.h
.In this way, we also could do anything we want in the new intrinsic API set.
This is maybe our last time to update the naming (maybe overloading version only), argument number and order, so we need to have comprehensive consideration and review all proposals in the issue list.
Both proposals may require a schedule for API freeze day, and we also need to list what are missing now..
The text was updated successfully, but these errors were encountered: