[PATCH] D113237: [RISCV] Support I extension version 2.1

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

In D113237#3172492 <https://reviews.llvm.org/D113237#3172492>, @achieveartificialintelligence wrote:

> In D113237#3124232 <https://reviews.llvm.org/D113237#3124232>, @luismarques wrote:
>
>> In D113237#3124188 <https://reviews.llvm.org/D113237#3124188>, @luismarques wrote:
>>
>>> Should we also support `fence.i` with RVI 2.0, without zifencei?
>>
>> Here's a proposed path forwards:
>>
>> - Patch 1: Add support for RVI 2.1 and related changes. Keep RVI 2.0 as the default.
>> - Patch 2: Add warnings for uses of the removed `fence.i`, CSRs and counters when using RVI 2.0 (at least when doing so by default; arguably this should error out instead when explicitly specifying version 2.0.)
>> - Patch 3: Start defaulting to RVI 2.1. IMO this should only be committed after patch 2 being in effect for a while.
>
> I tried to make I-ext imply `zifencei`, `zicsr`, and `zihintpause` automatically. BTW `zihintpause` is implemented in D93019 <https://reviews.llvm.org/D93019>.

It's a fairly minor detail, but I think implying zihintpause automatically is semantically different to what Luis proposed (as zihintpause wasn't in RVI 2.0, it shouldn't be implied by default like zifencei and zicsr should).

Luis' proposed series of patches makes a lot of sense to me, but a complicating factor is I'm not sure we have a super clean way to support different versions of an extension at the same time.

For instance, to make this patch match Luis' suggested "patch 1" you'd need to be able to specify rvi2.1 and then have it not automatically included zifencei and zicsr. Ideally the version emitted in build attributes would stay as 2.0 unless you explicitly selected 2.1 as well. This patch would then also not need to update test cases to include +zifencei and +zicsr.

We should discuss this in the RISC-V LLVM call today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113237