[PATCH] D29807: FileCheck-ize some tests in test/CodeGen/X86/

[Jorge Gorbe via Phabricator via llvm-commits]

jgorbe added a comment.

Some responses to @chandlerc 's previous comments.



================
Comment at: test/CodeGen/X86/2003-11-03-GlobalBool.ll:2
+; RUN: llc < %s -march=x86 | FileCheck %s
+; CHECK-NOT: {{.byte[[:space:]]*true}}
 
----------------
chandlerc wrote:
> You can use FileCheck's patterns for this:
> 
>   ; CHECK-NOT: .byte true
> 
> The whitespace is already not significant in FileCheck.
> 
> Also, I'd put this just after the global declaration below.
Done, thanks!


================
Comment at: test/CodeGen/X86/2004-02-13-FrameReturnAddress.ll:10-11
         ret i8* %X
+; CHECK-LABEL: test1
+; CHECK: (%esp
 }
----------------
chandlerc wrote:
> Generally you should put the `-LABEL` right at the top of the area you want to test, and you should make sure it won't spurriously match other strings. For example:
> 
>   define i8* @test1() {
>   ; CHECK-LABEL: define i8* @test1()
> 
> Also, you'll want to look at what 'llc' produces here and try to understand what the grep pattern above is really looking for `(%esp` seems to miss a lot of context, and now that this test is using FileCheck we want to try to get a reasonably good guess at what the original test was trying to detect.
llc won't output "define i8* @test1()" so that particular example wouldn't work here, right? I added a colon to match "test1:" as in the actual x86 assembly label.

As for the pattern matching `(%esp`, I preferred to err on the side of keeping existing behavior so I wouldn't make the tests more brittle by accident, but I guess we can CHECK for the whole `movl (%esp), %eax` that returns the return address. What do you think?


================
Comment at: test/CodeGen/X86/2004-02-14-InefficientStackPointer.ll:5-6
         ret i32 %X
+; CHECK-LABEL: test
+; CHECK-NOT: {{sub.*esp}}
 }
----------------
chandlerc wrote:
> You should be able to add a triple to pin down the particular assembly format and then look for `sub %esp` without having to use a regular expresssion.
Changed the check to just `sub %esp` so we don't use a regex.

There's already -march=x86. Do we need to specify the whole triple? I can add -x86-asm-syntax=att if we don't want to rely on it being the default.


https://reviews.llvm.org/D29807