[PATCH] D143513: [DebugInfo] Allow parsing line tables aligned to 4 or 8-byte boundaries

Fri Mar 17 01:04:55 PDT 2023

jhenderson added inline comments.

================
Comment at: llvm/test/tools/llvm-dwarfdump/ARM/aligned_line_tables.s:13-15
+// This test is based on a real example from ARM C/C++ Compiler.
+// It verifies llvm-dwarfdump is able to dump line tables even if they've been
+// placed at aligned offsets.
----------------
FWIW, I'd use `////` here, much like I'd use `##` to distinguish it from test code. Same goes with other test comments later on.

================
Comment at: llvm/test/tools/llvm-dwarfdump/ARM/aligned_line_tables.s:19
+// MULT4: Address            Line   Column File   ISA Discriminator Flags
+// MULT4: ------------------ ------ ------ ------ --- ------------- -------------
+// MULT4: 0x0000000000000000      1      0      1   0             0  is_stmt end_sequence
----------------
Here and throughout, can you use CHECK-NEXT (well, MULT4-NEXT etc)?

================
Comment at: llvm/test/tools/llvm-dwarfdump/ARM/aligned_line_tables.s:29
+// MULT4: min_inst_length: 2
+// MULT4-NOT: warning:
+// MULT4: default_is_stmt: 1
----------------
Why are you specifically testing that there is no warning at this specific point in the prologue printing?

================
Comment at: llvm/test/tools/llvm-dwarfdump/ARM/aligned_line_tables.s:49
+// LUNALIGN: 00000027 N .Ltable0_end
+// UNALIGN: warning:
+
----------------
I recommend you match the full warning message here. Otherwise, it might match another warning that isn't relevant. Also, it helps readers to know what warning you are actually testing for.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143513