[PATCH] D34764: Introduce FileEdit utility

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 28 11:36:06 PDT 2017


zturner added a comment.

In https://reviews.llvm.org/D34764#794154, @chandlerc wrote:

> In https://reviews.llvm.org/D34764#794086, @rnk wrote:
>
> > @chandlerc I think we have use cases for file splitting in the linker. Many interesting test cases require two object files, and it's annoying to have to make a separate file in %S/Inputs just for this. Consider lld/test/COFF/reloc-discarded.s. I recently added this and I need to make a COFF object that defines a global in a comdat. This is the RUN: line I used to do that:
> >
> >   # RUN: echo -e '.section .bss,"bw",discard,main_global\n.global main_global\n main_global:\n .long 0' | \
> >   # RUN:     llvm-mc - -filetype=obj -o %t1.obj -triple x86_64-windows-msvc
> >
> >
> > That's pretty unreadable.
>
>
> FWIW, I agree this is awful.
>
> I'm not as fussed as you seem to be about having separate files... but OK.
>
> > I would much rather write:
> > 
> >   # RUN: FileEdit %s %t
> >   # RUN: llvm-mc %t/a.s -o %t/a.obj -filetype=obj -triple x86_64-windows-msvc
> >   # RUN: llvm-mc %t/b.s -o %t/b.obj -filetype=obj -triple x86_64-windows-msvc
> >   # RUN: lld-link %t/a.obj %t/b.obj -entry ...
> >   
> >   # SPLIT: a.s
> >   .section .bss,"bw",discard,main_global
> >   .globl main_global
> >   main_global:
> >       .long 0
> >   
> >   # SPLIT: b.s
> >   .section .bss,"bw",discard,main_global
> >   .globl main_global
> >   main_global:
> >       .long 0
> >   ...
> > 
> > 
> > Editors would enable assembly syntax highlighting, even. You can apply the same technique to Clang tests that need headers.
>
> I just don't think we need a new syntax or tool here.
>
> We can run the preprocessor to create a header file, and then include it. We can use the preprocessor to have a single source file and compile it in N different ways.
>
> We could even use preprocessed assembly to solve the issue you cite in the `lld` test suite. I'd honestly rather continue using the *existing* file manipulation syntaxes rather than adding yet another magic comment that actually does semantic splitting.


When I went to edit some of the tests, I came up with a pattern that might be more agreeable than having a magic directive to spit out files?  I ended up taking this test:

  ; RUN: sed -e s/.T1:// %s | not opt -lint -disable-output 2>&1 | FileCheck --check-prefix=CHECK1 %s
  ; RUN: sed -e s/.T2:// %s | not opt -lint -disable-output 2>&1 | FileCheck --check-prefix=CHECK2 %s
  
  target triple = "x86_64-pc-windows-msvc"
  
  declare void @f()
  
  ;T1: declare i8* @llvm.eh.exceptionpointer.p0i8(i32)
  ;T1:
  ;T1: define void @test1() personality i32 (...)* @__CxxFrameHandler3 {
  ;T1:   call i8* @llvm.eh.exceptionpointer.p0i8(i32 0)
  ;T1:   ret void
  ;T1: }
  ;CHECK1: Intrinsic has incorrect argument type!
  ;CHECK1-NEXT: i8* (i32)* @llvm.eh.exceptionpointer.p0i8
  
  ;T2: declare i8* @llvm.eh.exceptionpointer.p0i8(token)
  ;T2:
  ;T2: define void @test2() personality i32 (...)* @__CxxFrameHandler3 {
  ;T2:   call i8* @llvm.eh.exceptionpointer.p0i8(token undef)
  ;T2:   ret void
  ;T2: }
  ;CHECK2: eh.exceptionpointer argument must be a catchpad
  ;CHECK2-NEXT:  call i8* @llvm.eh.exceptionpointer.p0i8(token undef)
  
  declare i32 @__CxxFrameHandler3(...)

and re-writing it as:

  ; RUN: FileEdit -keep-prefix=TEST1: %s | not opt -lint -disable-output 2>&1 | FileCheck --check-prefix=CHECK1 %s
  ; RUN: FileEdit -keep-prefix=TEST2: %s | not opt -lint -disable-output 2>&1 | FileCheck --check-prefix=CHECK2 %s
  
  TEST1: target triple = "x86_64-pc-windows-msvc"
  TEST1: declare void @f()
  TEST1: declare i8* @llvm.eh.exceptionpointer.p0i8(i32)
  TEST1: 
  TEST1: define void @test1() personality i32 (...)* @__CxxFrameHandler3 {
  TEST1:   call i8* @llvm.eh.exceptionpointer.p0i8(i32 0)
  TEST1:   ret void
  TEST1: }
  TEST1: declare i32 @__CxxFrameHandler3(...)
  
  CHECK1: Intrinsic has incorrect argument type!
  CHECK1-NEXT: i8* (i32)* @llvm.eh.exceptionpointer.p0i8
  
  TEST2: target triple = "x86_64-pc-windows-msvc"
  TEST2: declare void @f()
  TEST2: declare i8* @llvm.eh.exceptionpointer.p0i8(token)
  TEST2: 
  TEST2: define void @test2() personality i32 (...)* @__CxxFrameHandler3 {
  TEST2:   call i8* @llvm.eh.exceptionpointer.p0i8(token undef)
  TEST2:   ret void
  TEST2: }
  TEST2: declare i32 @__CxxFrameHandler3(...)
  
  CHECK2: eh.exceptionpointer argument must be a catchpad
  CHECK2-NEXT:  call i8* @llvm.eh.exceptionpointer.p0i8(token undef)

Perhaps just having a standardized way of saying "drop everything in the file that doesn't start with X, and for those that do, also drop the X" is sufficient?


https://reviews.llvm.org/D34764





More information about the llvm-commits mailing list