[PATCH] D61996: [llvm-objdump]Improve testing of some switches #2

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: test/tools/llvm-objdump/X86/Inputs/source-interleave.ll:2
+; ModuleID = 'source-interleave-x86_64.bc'
+source_filename = "source-interleave-x86_64.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
----------------
jhenderson wrote:
> MaskRay wrote:
> > Rename..
> Rename which bit? I can't rename the file to two different files (as things stand I've renamed X86/source-interleave-x86_64.ll to X86/source-interleave-x86_64.test). Also, my patch creation process may sometimes mess things up (sometimes it thinks things are modified, not added for example), but the final commit will actually be a rename.
> 
> If you're referring to renaming "source-interleave-x86_64.c" to something else, then no, because that needs to point at the .c file that this .ll was generated from (and that is unchanged).
Oh I thought you renamed `source-interleave-x86_64.c` to `source-interleave.c`. If the C source hasn't been renamed, why do you rename the `.ll` file?


================
Comment at: test/tools/llvm-objdump/X86/Inputs/source-interleave.ll:17
+define i32 @main() #0 !dbg !14 {
+entry:
+  %retval = alloca i32, align 4
----------------
I don't know how you feel about it, but I think these functions can just be empty. That is sufficient to emit debug info entries which will be used by the line table parser.


================
Comment at: test/tools/llvm-objdump/X86/source-interleave-missing-source.test:5
+# RUN: sed -e "s,SRC_COMPDIR,%t,g" %p/Inputs/source-interleave.ll > %t.ll
+# RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t2.ll
+
----------------
jhenderson wrote:
> MaskRay wrote:
> > I wanted to ask whether `sed -e 's,SRC_COMPDIR,%p,'` works on Windows, if it works, we don't need to use `%/p`.
> > 
> > In a *nix shell, `printf %s '\a\b'` => `\a\b`
> %p doesn't work in that string, because it becomes something like "C:\test\directory" and the \t and \d get interpreted as escape characters. See also https://reviews.llvm.org/D61856.
> 
> Note: I do most of my development on Windows, so I'll pick up issues like that.
That test said \t didn't work in double quotes. If \t doesn't work in single quotes, either, %/p seems fine.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61996