[PATCH] D117733: [RISCV] Update Privileged spec to version-20211203: Support Hypervisor Extention

Thu Feb 17 05:53:13 PST 2022

asb added a comment.

In D117733#3328033 <https://reviews.llvm.org/D117733#3328033>, @tangxingxin1008 wrote:

> In D117733#3266007 <https://reviews.llvm.org/D117733#3266007>, @asb wrote:
>
>> Thanks for the patch. Alongside the inline comments, two minor suggestions:
>>
>> - There's no need to split it out now I don't think, but the GPRAtomicMemOp->GPRMemZeroOffset is a change that could have been split out to a separate commit and reviewed separately.
>> - We don't do enough testing for this in general I don't think, but it would be good to add tests that attempting to use the RV64-only hypervisor instructions on RV32 produces an error.
>>
>> Once the inline comments are addressed and tests added for using the RV64 instructions on RV32, I think it should be ready to land.
>
> " There's no need to split it out now I don't think", do you mean the two commits [D117654 <https://reviews.llvm.org/D117654>, D117733 <https://reviews.llvm.org/D117733>]  should be to one commit?

Sorry that was a big ambiguous. I just meant that although splitting out the GPRMemZeroOffset change would be helpful, but not essential. But you've done it in D120017 <https://reviews.llvm.org/D120017> now anyway, which is helpful as it should unblock my other patch. Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117733