[PATCH] D57549: [llvm-objdump] - llvm-objdump can miss printing bytes at the end of a section.
    George Rimar via Phabricator via llvm-commits 
    llvm-commits at lists.llvm.org
       
    Fri Feb  1 00:25:21 PST 2019
    
    
  
grimar added a comment.
Herald added a project: LLVM.
Looks correct to me. Have a few comments/nits about the test case.
================
Comment at: test/tools/llvm-objdump/X86/bytes.test:8
+foo:
+.byte 't','h','i', 's',' ','i','s',' '
+.byte 'a',' ','t','e','s','t'
----------------
I would simplify to
`.ascii "this is a test"`
================
Comment at: test/tools/llvm-objdump/X86/bytes.test:11
+# .byte 0x74, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20
+# .byte 0x61, 0x20, 0x74, 0x65, 0x73, 0x74
+
----------------
I am not sure this is a useful 2 lines of comments.
You are testing the ASCII output first of all.
================
Comment at: test/tools/llvm-objdump/X86/bytes.test:13
+
+.size foo, . - foo
+
----------------
You do not need this line I think.
================
Comment at: test/tools/llvm-objdump/X86/bytes.test:16
+# CHECK: foo:
+# CHECK:       0:       74 68 69 73 20 69 73 20         this is
+# CHECK:       8:       61 20 74 65 73 74               a test
----------------
So maybe just omit testing the bytes?
```
0:       {{*.}}         this is
8:       {{*.}}         a test
```
Repository:
  rL LLVM
CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57549/new/
https://reviews.llvm.org/D57549
    
    
More information about the llvm-commits
mailing list