[PATCH] D88355: [DWARFYAML] Make the opcode_base and the standard_opcode_lengths fields optional.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

To summarise my inline suggestions, I think the behaviour should be:

1. Neither field specified: values are derived from the version. If the version is unsupported, return max(? - maybe v4 instead, to stop test behaviour changing in the future unexpectedly) supported version values.
2. Both fields specified: do as requested.
3. Just opcode_base specified, same as 1, except truncate the standard opcode lengths vector to the right length. If 0, use an empty standard opcode lengths vector.
4. Just standard opcode lengths specified: use length of specified lengths vector to derive opcode_base.



================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:556-560
+getStandardOpcodeLengths(uint16_t Version, Optional<uint8_t> OpcodeBase) {
+  if (OpcodeBase)
+    return std::vector<uint8_t>(*OpcodeBase > 0 ? *OpcodeBase - 1 : 0);
+  return std::vector<uint8_t>{0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1};
+}
----------------
You don't use the `Version` parameter, it looks like? I think you should - define the values based on the specified version, perhaps defaulting to v4 if the version is unknown (actually, I'd probably default to whatever the latest is, but that's academic, as v4 and v5 have the same standard opcodes).


================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:557-558
+getStandardOpcodeLengths(uint16_t Version, Optional<uint8_t> OpcodeBase) {
+  if (OpcodeBase)
+    return std::vector<uint8_t>(*OpcodeBase > 0 ? *OpcodeBase - 1 : 0);
+  return std::vector<uint8_t>{0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1};
----------------
I actually think the logic should be to truncate whatever the default vector is, rather than specifying a zero-filled vector.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml:716
+
+## Specify the opcode_base field (opcode_base!=0).
+
----------------



================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml:729
+
+## Specify the opcode_base field (opcode_base=0).
+
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88355