[Lldb-commits] [lldb] [lldb] Add a single bit constructor for RegisterFlags::Field (PR #69315)

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Wed Oct 18 04:07:01 PDT 2023


https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/69315

>From 1f5a161cb9265f2761e93d2352c65e382a44a1e5 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 17 Oct 2023 10:10:36 +0000
Subject: [PATCH 1/2] [lldb] Add a single bit constructor for RegisterField

This means you don't have to do RegisterField("", 0, 0),
you can do RegisterField("", 0).

Which is useful for testing and even more useful when we
are writing definitions of real registers which have 10s
of single bit fields.
---
 lldb/include/lldb/Target/RegisterFlags.h    |  6 ++++
 lldb/unittests/Target/RegisterFlagsTest.cpp | 35 +++++++++++----------
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index 8137fd27e99c2a2..ce3daa237194036 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -17,11 +17,17 @@ class RegisterFlags {
 public:
   class Field {
   public:
+    /// Where start is the least significant bit and end is the most
+    /// significant bit. The start bit must be <= the end bit.
     Field(std::string name, unsigned start, unsigned end)
         : m_name(std::move(name)), m_start(start), m_end(end) {
       assert(m_start <= m_end && "Start bit must be <= end bit.");
     }
 
+    /// Construct a single bit field.
+    Field(std::string name, unsigned bit)
+        : m_name(std::move(name)), m_start(bit), m_end(bit) {}
+
     /// Get size of the field in bits. Will always be at least 1.
     unsigned GetSizeInBits() const { return m_end - m_start + 1; }
 
diff --git a/lldb/unittests/Target/RegisterFlagsTest.cpp b/lldb/unittests/Target/RegisterFlagsTest.cpp
index 194e05959c16599..167e28d0cecb3bd 100644
--- a/lldb/unittests/Target/RegisterFlagsTest.cpp
+++ b/lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -16,7 +16,7 @@ using namespace lldb;
 TEST(RegisterFlagsTest, Field) {
   // We assume that start <= end is always true, so that is not tested here.
 
-  RegisterFlags::Field f1("abc", 0, 0);
+  RegisterFlags::Field f1("abc", 0);
   ASSERT_EQ(f1.GetName(), "abc");
   // start == end means a 1 bit field.
   ASSERT_EQ(f1.GetSizeInBits(), (unsigned)1);
@@ -51,11 +51,15 @@ static RegisterFlags::Field make_field(unsigned start, unsigned end) {
   return RegisterFlags::Field("", start, end);
 }
 
+static RegisterFlags::Field make_field(unsigned bit) {
+  return RegisterFlags::Field("", bit);
+}
+
 TEST(RegisterFlagsTest, FieldOverlaps) {
   // Single bit fields
-  ASSERT_FALSE(make_field(0, 0).Overlaps(make_field(1, 1)));
-  ASSERT_TRUE(make_field(1, 1).Overlaps(make_field(1, 1)));
-  ASSERT_FALSE(make_field(1, 1).Overlaps(make_field(3, 3)));
+  ASSERT_FALSE(make_field(0, 0).Overlaps(make_field(1)));
+  ASSERT_TRUE(make_field(1, 1).Overlaps(make_field(1)));
+  ASSERT_FALSE(make_field(1, 1).Overlaps(make_field(3)));
 
   ASSERT_TRUE(make_field(0, 1).Overlaps(make_field(1, 2)));
   ASSERT_TRUE(make_field(1, 2).Overlaps(make_field(0, 1)));
@@ -71,13 +75,13 @@ TEST(RegisterFlagsTest, PaddingDistance) {
   // (start bit is higher) field first and that they do not overlap.
 
   // [field 1][field 2]
-  ASSERT_EQ(make_field(1, 1).PaddingDistance(make_field(0, 0)), 0ULL);
+  ASSERT_EQ(make_field(1, 1).PaddingDistance(make_field(0)), 0ULL);
   // [field 1][..][field 2]
-  ASSERT_EQ(make_field(2, 2).PaddingDistance(make_field(0, 0)), 1ULL);
+  ASSERT_EQ(make_field(2, 2).PaddingDistance(make_field(0)), 1ULL);
   // [field 1][field 1][field 2]
-  ASSERT_EQ(make_field(1, 2).PaddingDistance(make_field(0, 0)), 0ULL);
+  ASSERT_EQ(make_field(1, 2).PaddingDistance(make_field(0)), 0ULL);
   // [field 1][30 bits free][field 2]
-  ASSERT_EQ(make_field(31, 31).PaddingDistance(make_field(0, 0)), 30ULL);
+  ASSERT_EQ(make_field(31, 31).PaddingDistance(make_field(0)), 30ULL);
 }
 
 static void test_padding(const std::vector<RegisterFlags::Field> &fields,
@@ -99,18 +103,18 @@ TEST(RegisterFlagsTest, RegisterFlagsPadding) {
 
   // Needs padding in between the fields, single bit.
   test_padding({make_field(17, 31), make_field(0, 15)},
-               {make_field(17, 31), make_field(16, 16), make_field(0, 15)});
+               {make_field(17, 31), make_field(16), make_field(0, 15)});
   // Multiple bits of padding.
   test_padding({make_field(17, 31), make_field(0, 14)},
                {make_field(17, 31), make_field(15, 16), make_field(0, 14)});
 
   // Padding before first field, single bit.
-  test_padding({make_field(0, 30)}, {make_field(31, 31), make_field(0, 30)});
+  test_padding({make_field(0, 30)}, {make_field(31), make_field(0, 30)});
   // Multiple bits.
   test_padding({make_field(0, 15)}, {make_field(16, 31), make_field(0, 15)});
 
   // Padding after last field, single bit.
-  test_padding({make_field(1, 31)}, {make_field(1, 31), make_field(0, 0)});
+  test_padding({make_field(1, 31)}, {make_field(1, 31), make_field(0)});
   // Multiple bits.
   test_padding({make_field(2, 31)}, {make_field(2, 31), make_field(0, 1)});
 
@@ -132,9 +136,8 @@ TEST(RegisterFieldsTest, ReverseFieldOrder) {
   ASSERT_EQ(0x56781234ULL, (unsigned long long)rf2.ReverseFieldOrder(0x12345678));
 
   // Many small fields.
-  RegisterFlags rf3("", 4,
-                    {make_field(31, 31), make_field(30, 30), make_field(29, 29),
-                     make_field(28, 28)});
+  RegisterFlags rf3(
+      "", 4, {make_field(31), make_field(30), make_field(29), make_field(28)});
   ASSERT_EQ(0x00000005ULL, rf3.ReverseFieldOrder(0xA0000000));
 }
 
@@ -167,7 +170,7 @@ TEST(RegisterFlagsTest, AsTable) {
             pos_wider.AsTable(100));
 
   // Single bit fields don't need to show start and end, just one of them.
-  RegisterFlags single_bit("", 4, {make_field(31, 31)});
+  RegisterFlags single_bit("", 4, {make_field(31)});
   ASSERT_EQ("| 31 | 30-0 |\n"
             "|----|------|\n"
             "|    |      |",
@@ -177,7 +180,7 @@ TEST(RegisterFlagsTest, AsTable) {
   RegisterFlags many_fields("", 4,
                             {RegisterFlags::Field("cat", 28, 31),
                              RegisterFlags::Field("pigeon", 20, 23),
-                             RegisterFlags::Field("wolf", 12, 12),
+                             RegisterFlags::Field("wolf", 12),
                              RegisterFlags::Field("x", 0, 4)});
   ASSERT_EQ("| 31-28 | 27-24 | 23-20  | 19-13 |  12  | 11-5 | 4-0 |\n"
             "|-------|-------|--------|-------|------|------|-----|\n"

>From 67213aaf0dd91b9bc555d2dcb0cddd3ce7809a31 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Wed, 18 Oct 2023 11:06:35 +0000
Subject: [PATCH 2/2] Expand comment and variable name.

---
 lldb/include/lldb/Target/RegisterFlags.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index ce3daa237194036..d98bc0263e35e23 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -24,9 +24,9 @@ class RegisterFlags {
       assert(m_start <= m_end && "Start bit must be <= end bit.");
     }
 
-    /// Construct a single bit field.
-    Field(std::string name, unsigned bit)
-        : m_name(std::move(name)), m_start(bit), m_end(bit) {}
+    /// Construct a field that occupies a single bit.
+    Field(std::string name, unsigned bit_position)
+        : m_name(std::move(name)), m_start(bit_position), m_end(bit_position) {}
 
     /// Get size of the field in bits. Will always be at least 1.
     unsigned GetSizeInBits() const { return m_end - m_start + 1; }



More information about the lldb-commits mailing list