[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8aea95f3cb4e: [lldb] Reland Use translated full ftag 
values (authored by mgorny).

Changed prior to commit:
  https://reviews.llvm.org/D91504?vs=306647=306849#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
   LinuxProcMapsTest.cpp
 
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -1,4 +1,6 @@
 # XFAIL: system-windows
+# Darwin uses the legacy behavior of reporting abridged ftag value,
+# it is covered by TestRegisters.py::fp_special_purpose_register_read.
 # XFAIL: system-darwin
 # REQUIRES: native && (target-x86 || target-x86_64)
 # RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
@@ -8,8 +10,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306647.
mgorny added a comment.

Skipped python test on non-Darwin, xfailed shell test on Darwin, added 
respective comments.


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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -1,4 +1,6 @@
 # XFAIL: system-windows
+# Darwin uses the legacy behavior of reporting abridged ftag value,
+# it is covered by TestRegisters.py::fp_special_purpose_register_read.
 # XFAIL: system-darwin
 # REQUIRES: native && (target-x86 || target-x86_64)
 # RUN: %clangxx_host %p/Inputs/x86-fp-write.cpp -o %t
@@ -8,8 +10,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D91504#2407397 , @labath wrote:

> Well... I think that will make the test fail on mac, as debugserver hasn't 
> been updated.
>
> This test comes from D12592 , and is 
> actually serves a very similar purpose to the test you just wrote. Normally, 
> I would say we don't need both of them, but since we still have debugserver 
> with the old behavior, and your new test is consistent with how we've 
> (you've) been writing other register tests, maybe we could just slap 
> `@skipUnlessDarwin` on TestRegisters.py, and put some comments (on both 
> tests) that explain the situation and cross-reference each other (?)

I could do that. I suppose it'd also be reasonably easy to make the Python test 
support both platforms with an `if`.


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Well... I think that will make the test fail on mac, as debugserver hasn't been 
updated.

This test comes from D12592 , and is actually 
serves a very similar purpose to the test you just wrote. Normally, I would say 
we don't need both of them, but since we still have debugserver with the old 
behavior, and your new test is consistent with how we've (you've) been writing 
other register tests, maybe we could just slap `@skipUnlessDarwin` on 
TestRegisters.py, and put some comments (on both tests) that explain the 
situation and cross-reference each other (?)


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny reopened this revision.
mgorny added a comment.
This revision is now accepted and ready to land.

Does this change to TestRegister.py look good? It makes the test pass for me 
(and seems logically correct).


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306403.
mgorny added a comment.

Update TestRegisters.py.


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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -14,9 +14,7 @@
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D91504#2405504 , @goncharov wrote:

> Hi @mgorny! http://lab.llvm.org:8011/#/builders/68/builds/2298 build failed. 
> Could you please fix the test?

I'm sorry about that. I'll look into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Mikhail Goncharov via Phabricator via lldb-commits]

goncharov added a comment.
Herald added a subscriber: JDevlieghere.

Hi @mgorny! http://lab.llvm.org:8011/#/builders/68/builds/2298 build failed. 
Could you please fix the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc43abf043692: [lldb] Use translated full ftag values 
(authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

ship it


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306360.
mgorny added a comment.

Declare `MMSRegComp` type explicitly to avoid warnings:

  In file included from 
/home/mgorny/llvm-project/llvm/tools/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp:11:
  In file included from 
/home/mgorny/llvm-project/llvm/tools/lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.h:12:
  
/home/mgorny/llvm-project/llvm/tools/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h:247:5:
 warning: anonymous types declared in an anonymous union are an extension 
[-Wnested-anon-types]
  struct {
  ^
  1 warning generated.


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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306356.
mgorny marked an inline comment as done.
mgorny added a comment.

Simplified FreeBSD & Linux code as @labath suggested. Also added the scope 
thingy to tests.


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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,73 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+std::array test_regs;
+for (int i = 0; i < x.value().st_reg_num; ++i)
+  test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
+EXPECT_EQ(
+AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
+x.value().tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const auto  : llvm::enumerate(tag_word_test_vectors)) {
+SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
+EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
+  }
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -14,9 +14,7 @@
 register read 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny marked 4 inline comments as done.
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:537
+uint16_t sw = m_xstate->fxsave.fstat;
+llvm::ArrayRef st_regs{m_xstate->fxsave.stmm, 8};
+reg_value.SetUInt16(AbridgedToFullTagWord(abridged_tw, sw, st_regs));

labath wrote:
> Is this really needed? I would have hoped that just passing 
> `m_xstate->fxsave.stmm` would be enough and that the c array constructor of 
> ArrayRef would do the right thing...
Indeed. I have missed that `ArrayRef` can figure the size of a C-style array 
out.


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks good. Some minor questions inline.




Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:523
+  case DBRegSet: {
+uint8_t *data = GetOffsetRegSetData(set, reg_info->byte_offset);
+FXSAVE *fpr = reinterpret_cast(m_fpr.data());

if you make data a `void*`, then you can remove the `reinterpret_cast` two 
lines below.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:528
+else
+  ::memcpy(GetOffsetRegSetData(set, reg_info->byte_offset),
+   reg_value.GetBytes(), reg_value.GetByteSize());

reuse data from above



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:537
+uint16_t sw = m_xstate->fxsave.fstat;
+llvm::ArrayRef st_regs{m_xstate->fxsave.stmm, 8};
+reg_value.SetUInt16(AbridgedToFullTagWord(abridged_tw, sw, st_regs));

Is this really needed? I would have hoped that just passing 
`m_xstate->fxsave.stmm` would be enough and that the c array constructor of 
ArrayRef would do the right thing...



Comment at: lldb/unittests/Process/Utility/RegisterContextTest.cpp:55
+  for (const TagWordTestVector  : tag_word_test_vectors) {
+std::array test_regs;
+for (int i = 0; i < x.st_reg_num; ++i)

add `SCOPED_TRACE` macro to make it clear which test case failed, when it 
fails. You can use llvm::enumerate to produce sequence numbers for the test 
cases...


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 306159.
mgorny retitled this revision from "[lldb] Use translated full ftag values 
[WIP]" to "[lldb] Use translated full ftag values".
mgorny added a comment.

Updated Linux to use pointer comparison and FreeBSD. It's ready for review now, 
though I'd use a better suggestion for FreeBSD.


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

https://reviews.llvm.org/D91504

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,65 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const TagWordTestVector  : tag_word_test_vectors) {
+std::array test_regs;
+for (int i = 0; i < x.st_reg_num; ++i)
+  test_regs[i] = st_regs[x.st_reg_num - i - 1];
+EXPECT_EQ(AbridgedToFullTagWord(x.tw_abridged, x.sw, test_regs), x.tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const TagWordTestVector  : tag_word_test_vectors)
+EXPECT_EQ(FullToAbridgedTagWord(x.tw), x.tw_abridged);
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -14,9 +14,7 @@
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 # CHECK-DAG: fstat = 0x8084
-# TODO: the following value is incorrect, it's a bug in the way
-# FXSAVE/XSAVE is interpreted
-# CHECK-DAG: ftag = 0x007f
+# 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:534
+  // TODO
+  if (reg_info->kinds[lldb::eRegisterKindLLDB] == lldb_ftag_x86_64) {
+uint8_t abridged_tw = *(uint8_t *)src;

mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > @labath, any suggestion what kind of test to use here? Maybe against 
> > > `_xstate->fxsave.ftag` address?
> > You mean, like, how to detect that we're dealing with the ftag register?
> > 
> > This approach seems reasonable to me..
> This = checking register number like in the code (in which I suppose we'd 
> have to move distinguishing amd64/i386 to ctor like `first_fpr`? Or checking 
> for actual pointer as I suggested in the comment?
I originally meant the register number approach, but I see what you mean now.

I think that going off of the address is perfectly fine...


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 305545.
mgorny added a comment.

Port NetBSD plugin.


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

https://reviews.llvm.org/D91504

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,65 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const TagWordTestVector  : tag_word_test_vectors) {
+std::array test_regs;
+for (int i = 0; i < x.st_reg_num; ++i)
+  test_regs[i] = st_regs[x.st_reg_num - i - 1];
+EXPECT_EQ(AbridgedToFullTagWord(x.tw_abridged, x.sw, test_regs), x.tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const TagWordTestVector  : tag_word_test_vectors)
+EXPECT_EQ(FullToAbridgedTagWord(x.tw), x.tw_abridged);
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -14,9 +14,7 @@
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 # CHECK-DAG: fstat = 0x8084
-# TODO: the following value is incorrect, it's a bug in the way
-# FXSAVE/XSAVE is interpreted
-# CHECK-DAG: ftag = 0x007f
+# CHECK-DAG: ftag = 0xea58
 # CHECK-DAG: fop = 0x0033
 # CHECK-DAG: fioff = [[FDIV]]
 # CHECK-DAG: fooff = [[ZERO]]
Index: lldb/test/Shell/Register/x86-64-fp-write.test
===
--- lldb/test/Shell/Register/x86-64-fp-write.test
+++ 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 305536.
mgorny added a comment.

Move the functions into a new file, add comment, use `llvm::ArrayRef`.


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

https://reviews.llvm.org/D91504

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,65 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+const std::array st_regs = {
+st_from_comp(0x8000, 0x4000), // +2.0
+st_from_comp(0x3f00, 0x), // 1.654785e-4932
+st_from_comp(0x, 0x), // +0
+st_from_comp(0x, 0x8000), // -0
+st_from_comp(0x8000, 0x7fff), // +inf
+st_from_comp(0x8000, 0x), // -inf
+st_from_comp(0xc000, 0x), // nan
+st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+const std::array tag_word_test_vectors{
+TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (const TagWordTestVector  : tag_word_test_vectors) {
+std::array test_regs;
+for (int i = 0; i < x.st_reg_num; ++i)
+  test_regs[i] = st_regs[x.st_reg_num - i - 1];
+EXPECT_EQ(AbridgedToFullTagWord(x.tw_abridged, x.sw, test_regs), x.tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (const TagWordTestVector  : tag_word_test_vectors)
+EXPECT_EQ(FullToAbridgedTagWord(x.tw), x.tw_abridged);
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -14,9 +14,7 @@
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 # CHECK-DAG: fstat = 0x8084
-# TODO: the following value is incorrect, it's a bug in the way
-# FXSAVE/XSAVE is interpreted
-# CHECK-DAG: ftag = 0x007f
+# CHECK-DAG: ftag = 0xea58
 # CHECK-DAG: fop = 0x0033
 # CHECK-DAG: fioff = [[FDIV]]
 # CHECK-DAG: fooff = [[ZERO]]
Index: lldb/test/Shell/Register/x86-64-fp-write.test
===
--- lldb/test/Shell/Register/x86-64-fp-write.test
+++ lldb/test/Shell/Register/x86-64-fp-write.test
@@ -8,8 +8,7 @@
 

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:534
+  // TODO
+  if (reg_info->kinds[lldb::eRegisterKindLLDB] == lldb_ftag_x86_64) {
+uint8_t abridged_tw = *(uint8_t *)src;

labath wrote:
> mgorny wrote:
> > @labath, any suggestion what kind of test to use here? Maybe against 
> > `_xstate->fxsave.ftag` address?
> You mean, like, how to detect that we're dealing with the ftag register?
> 
> This approach seems reasonable to me..
This = checking register number like in the code (in which I suppose we'd have 
to move distinguishing amd64/i386 to ctor like `first_fpr`? Or checking for 
actual pointer as I suggested in the comment?


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks reasonable to me.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:534
+  // TODO
+  if (reg_info->kinds[lldb::eRegisterKindLLDB] == lldb_ftag_x86_64) {
+uint8_t abridged_tw = *(uint8_t *)src;

mgorny wrote:
> @labath, any suggestion what kind of test to use here? Maybe against 
> `_xstate->fxsave.ftag` address?
You mean, like, how to detect that we're dealing with the ftag register?

This approach seems reasonable to me..



Comment at: lldb/source/Plugins/Process/Utility/RegisterContext_x86.h:381
 
+inline uint16_t AbridgedToFullTagWord(uint8_t abridged_tw, uint16_t sw, MMSReg 
*st_regs) {
+  // Tag word is using internal FPU register numbering rather than ST(i).

This is probably complex enough to deserve creating a cpp file (and a header 
comment explaining the high-level purpose of the function).



Comment at: lldb/source/Plugins/Process/Utility/RegisterContext_x86.h:381
 
+inline uint16_t AbridgedToFullTagWord(uint8_t abridged_tw, uint16_t sw, MMSReg 
*st_regs) {
+  // Tag word is using internal FPU register numbering rather than ST(i).

labath wrote:
> This is probably complex enough to deserve creating a cpp file (and a header 
> comment explaining the high-level purpose of the function).
Also, probably ArrayRef instead of the raw pointer.



Comment at: lldb/unittests/Process/Utility/RegisterContextTest.cpp:31
+
+std::array st_regs = {
+   st_from_comp(0x8000, 0x4000), // +2.0

Add const, for good measure (I see that will result in additional const 
qualifications in a bunch of places.


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

I'll port to other platforms once we agree on the approach.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:534
+  // TODO
+  if (reg_info->kinds[lldb::eRegisterKindLLDB] == lldb_ftag_x86_64) {
+uint8_t abridged_tw = *(uint8_t *)src;

@labath, any suggestion what kind of test to use here? Maybe against 
`_xstate->fxsave.ftag` address?


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

https://reviews.llvm.org/D91504

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

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
mgorny requested review of this revision.

Translate between abridged and full ftag values in order to expose
the latter in the gdb-remote protocol while the former are used by
FXSAVE/XSAVE...  This matches the gdb behavior.


https://reviews.llvm.org/D91504

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-64-fp-write.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/test/Shell/Register/x86-fp-write.test
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,65 @@
+//===-- RegisterContextTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/RegisterContext_x86.h"
+
+#include 
+
+using namespace lldb_private;
+
+struct TagWordTestVector {
+  uint16_t sw;
+  uint16_t tw;
+  uint8_t tw_abridged;
+  int st_reg_num;
+};
+
+constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
+  MMSReg ret = {};
+  ret.comp.mantissa = mantissa;
+  ret.comp.sign_exp = sign_exp;
+  return ret;
+}
+
+std::array st_regs = {
+	st_from_comp(0x8000, 0x4000), // +2.0
+	st_from_comp(0x3f00, 0x), // 1.654785e-4932
+	st_from_comp(0x, 0x), // +0
+	st_from_comp(0x, 0x8000), // -0
+	st_from_comp(0x8000, 0x7fff), // +inf
+	st_from_comp(0x8000, 0x), // -inf
+	st_from_comp(0xc000, 0x), // nan
+	st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+std::array tag_word_test_vectors{
+  TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
+  TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
+  TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
+  TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
+  TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
+  TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
+  TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
+  TagWordTestVector{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (TagWordTestVector  : tag_word_test_vectors) {
+std::array test_regs;
+for (int i = 0; i < x.st_reg_num; ++i)
+  test_regs[i] = st_regs[x.st_reg_num - i - 1];
+EXPECT_EQ(AbridgedToFullTagWord(x.tw_abridged, x.sw, test_regs.data()), x.tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (TagWordTestVector  : tag_word_test_vectors)
+EXPECT_EQ(FullToAbridgedTagWord(x.tw), x.tw_abridged);
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -8,9 +8,7 @@
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 # CHECK-DAG: fstat = 0x8084
-# TODO: the following value is incorrect, it's a bug in the way
-# FXSAVE/XSAVE is interpreted
-# CHECK-DAG: ftag = 0x007f
+# CHECK-DAG: ftag = 0xea58
 # CHECK-DAG: fop = 0x0033
 
 # CHECK-DAG: st{{(mm)?}}0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}
Index: lldb/test/Shell/Register/x86-64-fp-write.test
===
--- lldb/test/Shell/Register/x86-64-fp-write.test
+++ lldb/test/Shell/Register/x86-64-fp-write.test
@@ -8,8 +8,7 @@
 register write fctrl 0x037b
 register write fstat