[PATCH] D146054: [RISCV] Add --print-supported-extensions support

Brandon Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 30 00:39:21 PDT 2023


4vtomat added a comment.

In D146054#4596571 <https://reviews.llvm.org/D146054#4596571>, @MaskRay wrote:

> In D146054#4587210 <https://reviews.llvm.org/D146054#4587210>, @4vtomat wrote:
>
>> In D146054#4586067 <https://reviews.llvm.org/D146054#4586067>, @MaskRay wrote:
>>
>>> I think the best place to test `RISCVISAInfo.cpp` is `llvm/unittests/Support/RISCVISAInfoTest.cpp`.
>>>
>>> `clang/test/Driver/print-supported-extensions.c` can test just a few lines (there will be some overlap with the testing in `llvm/unittests/Support/RISCVISAInfoTest.cpp`), so that changes to RISC-V extensions will generally not require updates to `clang/test/Driver/print-supported-extensions.c`
>>
>> The goal of this patch is to list the supported extensions and their versions by providing an option, so I guess `clang/test/Driver/print-supported-extensions.c` aims differently with `llvm/unittests/Support/RISCVISAInfoTest.cpp` which is testing the functionalities in `RISCVISAInfoTest.cpp`.
>> `clang/test/Driver/print-supported-extensions.c` only tracks the extensions added and the their version changes and `riscvExtensionsHelp` in `llvm/lib/Support/RISCVISAInfo.cpp` doesn't have any input or output as well as any side effect, it only reads `SupportedExtensions` and `SupportedExperimentalExtensions` and dump the information.
>> So I think `clang/test/Driver/print-supported-extensions.c` is enough for this patch?
>
> My comment is about: the test is placed at the wrong layer. See https://maskray.me/blog/2021-08-08-toolchain-testing#the-test-checks-at-the-wrong-layer
>
> I don't want that RISC-V development in LLVM causes repeated changes to `clang/test/Driver`.

Got it, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054



More information about the cfe-commits mailing list