Page MenuHomePhabricator

[DWARFYAML] Make the ExtLen field of extended opcodes optional.
ClosedPublic

Authored by Higuoxing on Sep 22 2020, 11:47 PM.

Details

Summary

This patch makes the 'ExtLen' field of extended opcodes optional. We
don't need to manually calculate it in the future.

Diff Detail

Event Timeline

Higuoxing created this revision.Sep 22 2020, 11:47 PM
Higuoxing requested review of this revision.Sep 22 2020, 11:47 PM
jhenderson added inline comments.Sep 23 2020, 1:36 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
627

Something's not quite right at the end of this number sequence, the dots don't line up with the words beneath. Do you actually need the dots?

628–636

Where you're not interested in the header, maybe we could simplify this to just say something like ^------------------------ header?

Higuoxing updated this revision to Diff 293670.Sep 23 2020, 1:49 AM
Higuoxing marked 2 inline comments as done.

Address review comments.

Thanks!

llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
627

0x20 is <SPC>. Hence, the number of dots on the right side is 15 (16 - 1). I think we don't actually need them.

628–636

Good idea!

jhenderson added inline comments.Sep 23 2020, 1:53 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
638

Thinking about it, I guess we need something on this line to show that we've reached the end of the content (i.e. to show we haven't written any bogus extra data). Maybe something as simple as {{ }} (with more than one space). Other ideas also welcome.

Higuoxing updated this revision to Diff 293675.Sep 23 2020, 2:05 AM
Higuoxing marked an inline comment as done.

Thanks!

jhenderson added inline comments.Sep 23 2020, 2:08 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml
638

I think you need more than one space in the pattern. Otherwise this will still match:
00b42401 11223344 which would be the output with data appearing after the thing you want to check.

Thinking about it, I don't think {{ }} (two spaces) will work without FileCheck's --strict-whitespace. You might just be better off adding the extra bit at the end of the line for this line only.

Higuoxing updated this revision to Diff 293685.Sep 23 2020, 3:19 AM
Higuoxing marked an inline comment as done.

Append --strict-whitespace option.
Use {{ }} to match more (2) spaces.

This revision is now accepted and ready to land.Sep 23 2020, 3:32 AM
MaskRay accepted this revision.Sep 23 2020, 9:17 AM
This revision was landed with ongoing or failed builds.Sep 23 2020, 11:20 PM
This revision was automatically updated to reflect the committed changes.