labath added a comment.

I really like how you've approached testing this. There's just one thing that 
bothers me. Using a .s file means it's going to be hard to make this test work 
on other OSs (and we should be able to test something like this in a cross-OS 
manner). All elf targets should probably be fine, but I am pretty sure darwin 
and windows will choke on this.

I think it would be better to write this as a C(++) file and move the register 
setting code to inline asm. That should be more-or-less supported on all 
platforms (when compiling with clang). You should be able to test that by 
trying a cross-compilation with clang (`clang -target x86_64-pc-windows -c 
foo.c`). I've tried it with a simplified snipped from your test and it seemed 
to work fine.

(Note that I still expect this test to fail on windows, because AFAIK we don't 
support reading these registers on windows yet, but it would be good if at 
least the source code compiled there.)



================
Comment at: lldb/lit/Register/x86-mm-xmm-read.test:1
+# REQUIRES: x86
+# RUN: %clang %p/Inputs/x86-mm-xmm-read.s -o %t
----------------
Does this mean "the host is an x86 system", or "x86 is a configured llvm 
target"? My impression is that in means the latter 
<https://github.com/llvm-mirror/lldb/blob/master/lit/lit.cfg.py#L49>, but we 
obviously want the former here.


================
Comment at: lldb/lit/Register/x86-mm-xmm-read.test:7-22
+# CHECK: mm0 = 0x0102030405060708
+# CHECK-NEXT: mm1 = 0x1112131415161718
+# CHECK-NEXT: mm2 = 0x2122232425262728
+# CHECK-NEXT: mm3 = 0x3132333435363738
+# CHECK-NEXT: mm4 = 0x4142434445464748
+# CHECK-NEXT: mm5 = 0x5152535455565758
+# CHECK-NEXT: mm6 = 0x6162636465666768
----------------
Maybe you could drop the `-NEXT`s here (or even replace them by `-DAG`s), 
because (I presume) you are not interested in the order the registers get 
printed, just their values.


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

https://reviews.llvm.org/D60325



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to