[PATCH] D149314: [RISCV] Remove support for attribute interrupt("user").

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 27 10:22:20 PDT 2023


aaron.ballman added a comment.

In D149314#4302342 <https://reviews.llvm.org/D149314#4302342>, @asb wrote:

> In D149314#4302312 <https://reviews.llvm.org/D149314#4302312>, @reames wrote:
>
>> In D149314#4302300 <https://reviews.llvm.org/D149314#4302300>, @aaron.ballman wrote:
>>
>>> In D149314#4302266 <https://reviews.llvm.org/D149314#4302266>, @asb wrote:
>>>
>>>> In D149314#4302203 <https://reviews.llvm.org/D149314#4302203>, @aaron.ballman wrote:
>>>>
>>>>> Is this a potentially breaking change that we need to call out for users to be aware of?
>>>>
>>>> We should mention this in the Clang release notes I think.
>>>
>>> Beyond that, we've got a process for what to do when considering potentially breaking changes, we should be following that: https://llvm.org/docs/DeveloperPolicy.html#making-potentially-breaking-changes Also, if we're all agreed this is potentially breaking, it really should have more review time and buy-in from the code owner.
>>
>> I don't think this change should count as "potentially breaking" in the sense of that document.  We're talking about an experimental feature for an experimental ISA extension which never got to ratification.  There's no hardware in the wild which implements this (to my knowledge).  Given the churn on the RISCV extension side, we've adopted a policy for experimental extensions (https://llvm.org/docs/RISCVUsage.html#experimental-extensions) which offers much less in the way of support.  I think we should release note it just to be friendly, but the process described in your link is significant overkill.
>
> +1 on this. The upstream (RISC-V side) process for the ISA extension lifecycle and their ratification is now properly established, and we gate not-yet-ratified things behind `-menable-experimental-extensions` going forward. This review perhaps could have been held open a bit longer to check there's no concerns, and thanks to Aaron for raising the question. But I think a release note only is the appropriate option here.

SGTM, thank you for the discussion!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149314/new/

https://reviews.llvm.org/D149314



More information about the cfe-commits mailing list