[PATCH] D105429: [JITLink][RISCV] Initial Support RISCV64 in JITLink

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

In D105429#2903448 <https://reviews.llvm.org/D105429#2903448>, @MaskRay wrote:

> In D105429#2903436 <https://reviews.llvm.org/D105429#2903436>, @lhames wrote:
>
>> In D105429#2903344 <https://reviews.llvm.org/D105429#2903344>, @MaskRay wrote:
>>
>>> Perhaps revert this patch since it may be good to fix formatting nits in the initial RISC-V JIT patch.
>>
>> Please don't do that. Formatting nits can easily be fixed in-tree. Getting the code out to people (and testers) is more important.
>
> The test issues and formatting are not the only problems. The tests insufficient. The absolute and branch relocation types are not tested.
>
> From the description it's clear this is a WIP and not blocking any project.
>
> For initial check-in of some architecture, I think we should set up a higher bar. It looks multiple fixups are not needed for the patch.
> It's just cleaner to revert it.

As long as the existing tests are passing it's not regressing anything, and represents a monotonic improvement in functionality. More test coverage is better, and a discussion of where testing can/should be improved is good, but at this stage I think this patch is a good fit for in-tree development. I would like it to remain there, and future development to happen in-tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105429