[PATCH] D139553: [llvm-objdump][RISCV] Use new common method to parse ARCH RISCV attribute

[Elena Lepilkina via Phabricator via llvm-commits]

eklepilkina added inline comments.


================
Comment at: llvm/lib/Object/ELFObjectFile.cpp:308
+    // Suppress version checking for experimental extensions to prevent erroring
+    // when getting any unknown version.
+    auto ParseResult = RISCVISAInfo::parseArchString(
----------------
jrtc27 wrote:
> s/version/extension/?
It doesn't check version of experimental extensions only. The extension name is checked.


================
Comment at: llvm/lib/Object/ELFObjectFile.cpp:311
+        *Attr, /*EnableExperimentalExtension=*/true,
+        /*ExperimentalExtensionVersionCheck=*/false,
+        /*IgnoreUnknown=*/true);
----------------
jrtc27 wrote:
> This means a file that has, say, v0p7, will disassemble as v1p0 rather than being ignored like an extension whose name isn't known?..
If it's experimental extension yes. This can be changed to use more strict rules, but not sure that this is needed for experimental extensions.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/RISCV/extensions.test:1
 ## Handle RISCV extensions.
 ## Encode an zbc arch feature into an object file and try to decode it.
----------------
kito-cheng wrote:
> Failed to apply this patch locally due to missing this file, are you forgot to add a precommit patch for this?
I wasn't told not to load precommit patch at the same review. So yes, it exsits now only on my branch. I can't commit it in upstream, so I have asked a collegue to do this. Precommit patch wil be on master soon. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139553