This is an archive of the discontinued LLVM Phabricator instance.

Differential D61303

[lldb] [lit] Add write tests for MM/XMM/r8-r15 registers
ClosedPublic

Authored by mgorny on Apr 30 2019, 4:12 AM.

Details

Reviewers
labath
JDevlieghere
krytarowski
Commits
rG97799084947c: [lldb] [lit] Add write tests for r8-r15 & xmm8-xmm15 registers
rGb268a2a4af42: [lldb] [lit] Add write tests for MM/XMM registers
rL359682: [lldb] [lit] Add write tests for r8-r15 & xmm8-xmm15 registers
rLLDB359682: [lldb] [lit] Add write tests for r8-r15 & xmm8-xmm15 registers
rLLDB359681: [lldb] [lit] Add write tests for MM/XMM registers
rL359681: [lldb] [lit] Add write tests for MM/XMM registers

Diff Detail

Event Timeline

mgorny created this revision.Apr 30 2019, 4:12 AM
labath accepted this revision.Apr 30 2019, 5:31 AM

The assembly looks perfectly fine to me. The only thing I'd consider changing is writing the displacements in the movq ... (%rbx) instructions in hex.

lldb/lit/Register/Inputs/x86-mm-xmm-write.cpp
11

It looks like mm_fill is unused.

lldb/lit/Register/x86-mm-xmm-write.test
28

When looking at the jenkins output from the failed gp-write run, I realized that the failure message isn't very helpful because FileCheck suggests the "register write" line above as a "possible intended match".

I think it may be possible to improve on that by adding a CHECK: process continue here so that all subsequent checks only look at the process output, and not the commands setting the values.

This revision is now accepted and ready to land.Apr 30 2019, 5:31 AM
mgorny updated this revision to Diff 197306.Apr 30 2019, 6:11 AM
mgorny marked 2 inline comments as done.

Switched to hex offsets, and added match for 'process continue'.

mgorny updated this revision to Diff 197355.Apr 30 2019, 9:42 AM
mgorny marked an inline comment as done.
mgorny retitled this revision from [lldb] [lit] Add write tests for MM/XMM registers to [lldb] [lit] Add write tests for MM/XMM/r8-r15 registers.
mgorny edited the summary of this revision. (Show Details)

Inlined mm_fill, and added tests for r8-r15 & xmm8-xmm15.

JDevlieghere added inline comments.Apr 30 2019, 10:17 AM
lldb/lit/Register/x86-64-write.test
1 ↗(On Diff #197355)

This fails on Darwin for me:

# CHECK-DAG: xmm8 = 0x030507090b0d0f00020406080a0c0e01
             ^
<stdin>:38:1: note: scanning from here
r8 = 0x0102030405060708
^
<stdin>:46:1: note: possible intended match here
xmm8 = 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f
mgorny marked an inline comment as done.Apr 30 2019, 10:22 AM
mgorny added inline comments.
lldb/lit/Register/Inputs/x86-mm-xmm-write.cpp
11

It's used to fill xmm_fill ;-). Should I inline it?

lldb/lit/Register/x86-64-write.test
1 ↗(On Diff #197355)

Could you run it without FileCheck, and paste the full output? Manually would be something alike:

clang++ Inputs/x86-64-write.cpp
lldb -b -s x86-64-write.test a.out
JDevlieghere added inline comments.Apr 30 2019, 10:31 AM
lldb/lit/Register/x86-64-write.test
1 ↗(On Diff #197355)

Sure, here's the output: https://reviews.llvm.org/P8141

mgorny updated this revision to Diff 197376.Apr 30 2019, 10:55 AM

Ok, I see that Darwin doesn't implement writing to xmm* registers at the moment. Added XFAIL for it.

labath marked an inline comment as done.Apr 30 2019, 11:05 AM
labath added inline comments.
lldb/lit/Register/Inputs/x86-mm-xmm-write.cpp
11

What I actually meant here was that I was expecting mm_fill to be used to initialize the mm registers, but you're initializing them with xmm_fill instead. However, that's not really important, as both are technically correct.

JDevlieghere accepted this revision.Apr 30 2019, 11:06 AM
Closed by commit rL359681: [lldb] [lit] Add write tests for MM/XMM registers (authored by mgorny). · Explain WhyMay 1 2019, 6:54 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2019, 6:55 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript

Revision Contents

PathSize
lldb/
lit/
Register/
Inputs/
68 lines
47 lines

Diff 197306

lldb/lit/Register/Inputs/x86-mm-xmm-write.cpp

  • This file was added.
#include <cinttypes>
#include <cstdint>
#include <cstdio>
struct alignas(16) xmm_t {
uint64_t a, b;
};
int main() {
constexpr uint64_t mm_fill = 0x0F0F0F0F0F0F0F0F;
constexpr xmm_t xmm_fill = { mm_fill, mm_fill };
labathUnsubmitted
Done
ReplyInline Actions

It looks like mm_fill is unused.

labath: It looks like `mm_fill` is unused.
mgornyAuthorUnsubmitted
Done
ReplyInline Actions

It's used to fill xmm_fill ;-). Should I inline it?

mgorny: It's used to fill `xmm_fill` ;-). Should I inline it?
labathUnsubmitted
Done
ReplyInline Actions

What I actually meant here was that I was expecting mm_fill to be used to initialize the mm registers, but you're initializing them with xmm_fill instead. However, that's not really important, as both are technically correct.

labath: What I actually meant here was that I was expecting `mm_fill` to be used to initialize the `mm`…
uint64_t mm[8];
xmm_t xmm[8];
asm volatile(
"movq %2, %%mm0\n\t"
"movq %2, %%mm1\n\t"
"movq %2, %%mm2\n\t"
"movq %2, %%mm3\n\t"
"movq %2, %%mm4\n\t"
"movq %2, %%mm5\n\t"
"movq %2, %%mm6\n\t"
"movq %2, %%mm7\n\t"
"\n\t"
"movaps %2, %%xmm0\n\t"
"movaps %2, %%xmm1\n\t"
"movaps %2, %%xmm2\n\t"
"movaps %2, %%xmm3\n\t"
"movaps %2, %%xmm4\n\t"
"movaps %2, %%xmm5\n\t"
"movaps %2, %%xmm6\n\t"
"movaps %2, %%xmm7\n\t"
"\n\t"
"int3\n\t"
"\n\t"
"lea %0, %%rbx\n\t"
"movq %%mm0, 0x00(%%rbx)\n\t"
"movq %%mm1, 0x08(%%rbx)\n\t"
"movq %%mm2, 0x10(%%rbx)\n\t"
"movq %%mm3, 0x18(%%rbx)\n\t"
"movq %%mm4, 0x20(%%rbx)\n\t"
"movq %%mm5, 0x28(%%rbx)\n\t"
"movq %%mm6, 0x30(%%rbx)\n\t"
"movq %%mm7, 0x38(%%rbx)\n\t"
"\n\t"
"lea %1, %%rbx\n\t"
"movaps %%xmm0, 0x00(%%rbx)\n\t"
"movaps %%xmm1, 0x10(%%rbx)\n\t"
"movaps %%xmm2, 0x20(%%rbx)\n\t"
"movaps %%xmm3, 0x30(%%rbx)\n\t"
"movaps %%xmm4, 0x40(%%rbx)\n\t"
"movaps %%xmm5, 0x50(%%rbx)\n\t"
"movaps %%xmm6, 0x60(%%rbx)\n\t"
"movaps %%xmm7, 0x70(%%rbx)\n\t"
: "=m"(mm), "=m"(xmm)
: "m"(xmm_fill)
: "%rbx", "%mm0", "%mm1", "%mm2", "%mm3", "%mm4", "%mm5", "%mm6", "%mm7",
"%xmm0", "%xmm1", "%xmm2", "%xmm3", "%xmm4", "%xmm5", "%xmm6", "%xmm7"
);
for (int i = 0; i < 8; ++i)
printf("mm%d = 0x%016" PRIx64 "\n", i, mm[i]);
for (int i = 0; i < 8; ++i)
printf("xmm%d = 0x%016" PRIx64 "%016" PRIx64 "\n", i, xmm[i].b, xmm[i].a);
return 0;
}

lldb/lit/Register/x86-mm-xmm-write.test

  • This file was added.
# XFAIL: system-darwin
# XFAIL: system-windows
# REQUIRES: native && (target-x86 || target-x86_64) && native-cpu-sse
# RUN: %clangxx %p/Inputs/x86-mm-xmm-write.cpp -o %t
# RUN: %lldb -b -s %s %t | FileCheck %s
process launch
register write mm0 0x0102030405060708
register write mm1 0x1112131415161718
register write mm2 0x2122232425262728
register write mm3 0x3132333435363738
register write mm4 0x4142434445464748
register write mm5 0x5152535455565758
register write mm6 0x6162636465666768
register write mm7 0x7172737475767778
register write xmm0 "{0x01 0x0e 0x0c 0x0a 0x08 0x06 0x04 0x02 0x00 0x0f 0x0d 0x0b 0x09 0x07 0x05 0x03}"
register write xmm1 "{0x11 0x1e 0x1c 0x1a 0x18 0x16 0x14 0x12 0x10 0x1f 0x1d 0x1b 0x19 0x17 0x15 0x13}"
register write xmm2 "{0x21 0x2e 0x2c 0x2a 0x28 0x26 0x24 0x22 0x20 0x2f 0x2d 0x2b 0x29 0x27 0x25 0x23}"
register write xmm3 "{0x31 0x3e 0x3c 0x3a 0x38 0x36 0x34 0x32 0x30 0x3f 0x3d 0x3b 0x39 0x37 0x35 0x33}"
register write xmm4 "{0x41 0x4e 0x4c 0x4a 0x48 0x46 0x44 0x42 0x40 0x4f 0x4d 0x4b 0x49 0x47 0x45 0x43}"
register write xmm5 "{0x51 0x5e 0x5c 0x5a 0x58 0x56 0x54 0x52 0x50 0x5f 0x5d 0x5b 0x59 0x57 0x55 0x53}"
register write xmm6 "{0x61 0x6e 0x6c 0x6a 0x68 0x66 0x64 0x62 0x60 0x6f 0x6d 0x6b 0x69 0x67 0x65 0x63}"
register write xmm7 "{0x71 0x7e 0x7c 0x7a 0x78 0x76 0x74 0x72 0x70 0x7f 0x7d 0x7b 0x79 0x77 0x75 0x73}"
process continue
# CHECK: process continue
labathUnsubmitted
Done
ReplyInline Actions

When looking at the jenkins output from the failed gp-write run, I realized that the failure message isn't very helpful because FileCheck suggests the "register write" line above as a "possible intended match".

I think it may be possible to improve on that by adding a CHECK: process continue here so that all subsequent checks only look at the process output, and not the commands setting the values.

labath: When looking at the jenkins output from the failed gp-write run, I realized that the failure…
# CHECK-DAG: mm0 = 0x0102030405060708
# CHECK-DAG: mm1 = 0x1112131415161718
# CHECK-DAG: mm2 = 0x2122232425262728
# CHECK-DAG: mm3 = 0x3132333435363738
# CHECK-DAG: mm4 = 0x4142434445464748
# CHECK-DAG: mm5 = 0x5152535455565758
# CHECK-DAG: mm6 = 0x6162636465666768
# CHECK-DAG: mm7 = 0x7172737475767778
# CHECK-DAG: xmm0 = 0x030507090b0d0f00020406080a0c0e01
# CHECK-DAG: xmm1 = 0x131517191b1d1f10121416181a1c1e11
# CHECK-DAG: xmm2 = 0x232527292b2d2f20222426282a2c2e21
# CHECK-DAG: xmm3 = 0x333537393b3d3f30323436383a3c3e31
# CHECK-DAG: xmm4 = 0x434547494b4d4f40424446484a4c4e41
# CHECK-DAG: xmm5 = 0x535557595b5d5f50525456585a5c5e51
# CHECK-DAG: xmm6 = 0x636567696b6d6f60626466686a6c6e61
# CHECK-DAG: xmm7 = 0x737577797b7d7f70727476787a7c7e71
# CHECK: Process {{[0-9]+}} exited with status = 0