[llvm-commits] [Review request] test/CodeGen/X86: Add cases for Win64 to 67 tests

Michael Spencer bigcheesegs at gmail.com
Mon Nov 22 16:31:15 PST 2010


On Mon, Nov 22, 2010 at 2:49 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> Hello guys!
>
> llc infers -march=x86-64 as -mtriple=x86_64-(mingw32|win32).
> Then many tests in CodeGen/X86 would fail due to ABI difference
> between AMD64 and Win64.
> (or Win64 codegen's inefficiency)
>
> The principle to rewrite tests is as below;
>
>  - Tests should not hide possible win64's behavior.
>    They could exclude Win64 when they were obviously nonsense.
>  - Tests should not depend on hosts. Win64's failure should be
> detected also on other platforms.
>    When a test has -mtriple=x86_64-linux, it should have also
> -mtriple=x86_64-win32.
>  - Some tests might be dubious for Win64. I took two cases.
>    - Mark  them as XFAIL: mingw, win32, to detect when they were fixed.
>    - Add *as-is* pattern to tests. You shall know they are really ugly. :D
>
> Guys, please, I would like everyone to help me to brush up my patches.
> Feel free for yourself to commit one when you might be self-approved.
>
> * 0001-test-CodeGen-X86-fold-mul-lohi.ll-FileCheck-ize-and-.patch
>
>  Win64 tends to emit "lea (%rip)". I excluded it from win64 tests.
>
> * 0002-test-CodeGen-X86-red-zone.ll-Add-explicit-mtriple-x8.patch
>
>  Win64 doesn't use redzone stuff.

Looks good to me.

> * 0003-test-CodeGen-X86-sse_reload_fold.ll-FileCheck-ize-an.patch
>
>  I don't understand what it intends. I am happy if someone explain to me. :)

Looks good, except |& can be changed to |. I don't really know what
this is testing.

> * 0004-test-CodeGen-X86-store_op_load_fold2.ll-Add-a-test-f.patch
>
>  Alignment of 13th member in the struct makes difference.

I would feel better if I actually knew what was being tested, but if
the index is indeed significant, this looks good.

> * 0005-test-CodeGen-X86-v2f32.ll-Fix-missing-tests-and-add-.patch
>
>  I re-enabled 3 tests and added *as-is* Win64's patterns.

This looks correct to me. Stupid Windows x86-64 only using 4 GPRs for
argument passing :(. Although I think we can get rid of the shadow
stack because this is a leaf function.

> * 0006-test-CodeGen-X86-Add-the-as-is-pattern-for-Win64.patch
>
>  I added *as-is* patterns for Win64. I intend for reviewrs to help to
> reduce their expressions.

This one is going to take a while to review :P.

> * 0007-test-CodeGen-X86-Mark-as-XFAIL-mingw-win32.-They-sho.patch
>
>  I thought they could be fixed in future. When one were fixed, "llc
> -mtriple=x86_64-win32" should be added.

I noticed you documented some of these. Can you document the rest and
please not only reference PR numbers?

> * 0008-test-CodeGen-X86-FileCheck-ize-with-relaxed-expressi.patch
>
>  Please watch out;
>    - Is my FileCheck-izing reasonable?
>    - Do relaxed patterns have any faults?

diff --git a/test/CodeGen/X86/2007-01-08-X86-64-Pointer.ll
b/test/CodeGen/X86/2007-01-08-X86-64-Pointer.ll
index de226a1..90e7fab 100644
--- a/test/CodeGen/X86/2007-01-08-X86-64-Pointer.ll
+++ b/test/CodeGen/X86/2007-01-08-X86-64-Pointer.ll
@@ -1,5 +1,8 @@
-; RUN: llc %s -o - -march=x86-64 | grep {(%rdi,%rax,8)}
-; RUN: llc %s -o - -march=x86-64 | not grep {addq.*8}
+; RUN: llc < %s -mtriple=x86_64-linux | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-win32 | FileCheck %s
+; CHECK-NOT: {{addq.*8}}
+; CHECK:     ({{%rdi,%rax|%rcx,%rax}},8)
+; CHECK-NOT: {{addq.*8}}

The patterns can be changed to.

+; CHECK-NOT: addq{{.*}}8
+; CHECK:     (%r{{di|cx}},%rax,8)
+; CHECK-NOT: addq{{.*}}8

A bunch of others are like this too. Put the minimum amount of text in
the brackets.

Only Linux and Windows are handled, not Darwin. I agree that all
testers should test code gen for more than just the host platform, but
we need to not exclude major platforms.

Other than that it looks fine, but I'm not sure if they are only
testing what the author wanted. I really wish people would document
their tests.

> * 0009-test-CodeGen-X86-FileCheck-ize-and-add-the-case-for-.patch
>
>  I added "WIN64:"(and similar) expressions to them.
>  I wonder it might be reasonable.

Adding WIN64 is OK if the test actually relies on how arguments are
passed or other ABI issues. Other than that, same comments as above.

> * 0010-test-CodeGen-X86-Relax-expressions-for-Win64.patch
>
>  They are FileCheck-ized on ToT. I tweaked them with relaxation to
> recognize Win64 ABI.

Same again.

> Have a happy testing! ...Takumi
>
>
>
> test/CodeGen/X86/2007-01-08-X86-64-Pointer.ll    |    7 ++-
> test/CodeGen/X86/2007-07-18-Vector-Extract.ll    |    6 +-
> test/CodeGen/X86/2008-06-13-VolatileLoadStore.ll |    9 ++-
> test/CodeGen/X86/add.ll                          |   11 ++--
> test/CodeGen/X86/avoid-lea-scale2.ll             |    5 +-
> test/CodeGen/X86/bitcast2.ll                     |   22 ++++++-
> test/CodeGen/X86/break-sse-dep.ll                |   15 ++--
> test/CodeGen/X86/byval.ll                        |    9 ++-
> test/CodeGen/X86/byval2.ll                       |   39 ++++++++++-
> test/CodeGen/X86/byval3.ll                       |   42 +++++++++++-
> test/CodeGen/X86/byval4.ll                       |   42 +++++++++++-
> test/CodeGen/X86/byval5.ll                       |   42 +++++++++++-
> test/CodeGen/X86/coalescer-commute2.ll           |   21 +++++-
> test/CodeGen/X86/codegen-prepare-extload.ll      |    5 +-
> test/CodeGen/X86/constant-pool-remat-0.ll        |   35 +++++++++-
> test/CodeGen/X86/constant-pool-sharing.ll        |    7 +-
> test/CodeGen/X86/convert-2-addr-3-addr-inc64.ll  |   12 +++-
> test/CodeGen/X86/fast-isel-cmp-branch.ll         |    7 +-
> test/CodeGen/X86/fast-isel-gep.ll                |   11 ++--
> test/CodeGen/X86/fold-mul-lohi.ll                |    5 +-
> test/CodeGen/X86/gather-addresses.ll             |   23 ++++---
> test/CodeGen/X86/h-register-store.ll             |   32 +++++++--
> test/CodeGen/X86/h-registers-0.ll                |   24 +++++++-
> test/CodeGen/X86/i128-ret.ll                     |    6 +-
> test/CodeGen/X86/i64-mem-copy.ll                 |    8 ++-
> test/CodeGen/X86/iabs.ll                         |    8 ++-
> test/CodeGen/X86/lea-3.ll                        |   15 +++--
> test/CodeGen/X86/lea.ll                          |    9 ++-
> test/CodeGen/X86/lsr-overflow.ll                 |    5 +-
> test/CodeGen/X86/lsr-reuse-trunc.ll              |    9 ++-
> test/CodeGen/X86/masked-iv-safe.ll               |   43 ++++++++++---
> test/CodeGen/X86/memcmp.ll                       |   21 +++---
> test/CodeGen/X86/mmx-copy-gprs.ll                |   10 ++-
> test/CodeGen/X86/movgs.ll                        |    7 +-
> test/CodeGen/X86/optimize-max-3.ll               |   11 ++--
> test/CodeGen/X86/peep-vector-extract-concat.ll   |   11 +++-
> test/CodeGen/X86/pmulld.ll                       |   12 +++-
> test/CodeGen/X86/red-zone.ll                     |    2 +-
> test/CodeGen/X86/red-zone2.ll                    |   11 ++-
> test/CodeGen/X86/remat-mov-0.ll                  |   15 ++--
> test/CodeGen/X86/scalar-min-max-fill-operand.ll  |   34 +++++++++-
> test/CodeGen/X86/select-aggregate.ll             |    7 +-
> test/CodeGen/X86/sse-align-0.ll                  |   25 +++++++-
> test/CodeGen/X86/sse-align-3.ll                  |   22 ++++++-
> test/CodeGen/X86/sse-align-7.ll                  |    8 ++-
> test/CodeGen/X86/sse-commute.ll                  |   12 +++-
> test/CodeGen/X86/sse-minmax.ll                   |    2 +
> test/CodeGen/X86/sse_reload_fold.ll              |    5 +-
> test/CodeGen/X86/stdarg.ll                       |   16 ++++-
> test/CodeGen/X86/store_op_load_fold2.ll          |    9 ++-
> test/CodeGen/X86/stride-nine-with-base-reg.ll    |   34 +++++++++-
> test/CodeGen/X86/stride-reuse.ll                 |   33 +++++++++-
> test/CodeGen/X86/subreg-to-reg-4.ll              |   11 +--
> test/CodeGen/X86/tailcallbyval64.ll              |   62 ++++++++++++++++-
> test/CodeGen/X86/tailcallstack64.ll              |    3 +-
> test/CodeGen/X86/test-shrink.ll                  |   21 +++---
> test/CodeGen/X86/unknown-location.ll             |    5 +-
> test/CodeGen/X86/use-add-flags.ll                |   15 ++--
> test/CodeGen/X86/v2f32.ll                        |   77 ++++++++++++++++++---
> test/CodeGen/X86/vec_cast.ll                     |    3 +-
> test/CodeGen/X86/vec_set-8.ll                    |    7 ++-
> test/CodeGen/X86/vec_set-F.ll                    |   22 +++++-
> test/CodeGen/X86/vec_shuffle-17.ll               |    7 ++-
> test/CodeGen/X86/vec_shuffle-37.ll               |    5 +-
> test/CodeGen/X86/widen_load-0.ll                 |    8 ++-
> test/CodeGen/X86/x86-64-malloc.ll                |    4 +-
> test/CodeGen/X86/xor.ll                          |    9 ++-
> 67 files changed, 884 insertions(+), 216 deletions(-)
>

Thanks again for doing this. All these tests now pass for me on MSVC
2010 32bit and MinGW32 on Windows 7 x64.

- Michael Spencer




More information about the llvm-commits mailing list