[PATCH] D112359: [RISCV] Unify depedency check and extension implication parsing logics

Fri Nov 5 04:09:42 PDT 2021

asb added a comment.

In D112359#3098960 <https://reviews.llvm.org/D112359#3098960>, @eopXD wrote:

> 

> Add version numbers for test case in `attribute-arch.s`

Thanks for updating the patch. Why change this test case? I thought those lines were verifying that .attribute arch without a version number was accepted but the output file included a version number. I'll admit that's not fully consistent with requiring the version number explicitly on the command-line. If a behaviour change is desired here, it would be good to test the case where version numbers aren't specified.

I think for a patch like this, if it at all possible it would be best to do the refactoring in a way that doesn't alter behaviour and then follow-up with separate patches that change any other behaviour (if desired). Then we can review and discuss the refactoring and any functional changes separately, without one discussion blocking the other.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112359