[DSLX] trace_fmt! support for signed values (e.g. s32) printed signed…

…-ly.

Important aid in DSL level debugging.

PiperOrigin-RevId: 455507157

docs_src/dslx_reference.md

@@ -1939,12 +1939,16 @@ dumping of current values to stdout. For example:
 
 fn shifty(x: u8, y: u3) -> u8 {
   let _ = trace_fmt!("x: {:x} y: {}", x, y);
+  // Note: y looks different as a negative number when the high bit is set.
+  let _ = trace_fmt!("y as s8: {}", y as s2);
   x << y
 }
 
 #![test]
 fn test_shifty() {
-  assert_eq(shifty(u8:0x42, u3:4), u8:0x20)
+  let _ = assert_eq(shifty(u8:0x42, u3:4), u8:0x20);
+  let _ = assert_eq(shifty(u8:0x42, u3:7), u8:0);
+  ()
 }
 ```
 
@@ -1955,6 +1959,9 @@ corresponding source position:
 [...]
 [ RUN UNITTEST  ] test_shifty
 I0510 14:31:17.516227 1247677 bytecode_interpreter.cc:994] x: 42 y: 4
+I0510 14:31:17.516227 1247677 bytecode_interpreter.cc:994] y as s8: 4
+I0510 14:31:17.516227 1247677 bytecode_interpreter.cc:994] x: 42 y: 7
+I0510 14:31:17.516227 1247677 bytecode_interpreter.cc:994] y as s8: -1
 [            OK ]
 [...]
 ```

xls/codegen/vast.cc

@@ -707,15 +707,16 @@ std::string Literal::Emit(LineInfo* line_info) const {
   LineInfoEnd(line_info, this);
   if (format_ == FormatPreference::kDefault) {
     XLS_CHECK_LE(bits_.bit_count(), 32);
-    return absl::StrFormat("%s", bits_.ToString(FormatPreference::kDecimal));
+    return absl::StrFormat("%s",
+                           bits_.ToString(FormatPreference::kUnsignedDecimal));
   }
-  if (format_ == FormatPreference::kDecimal) {
+  if (format_ == FormatPreference::kUnsignedDecimal) {
     std::string prefix;
     if (emit_bit_count_) {
       prefix = absl::StrFormat("%d'd", bits_.bit_count());
     }
     return absl::StrFormat("%s%s", prefix,
-                           bits_.ToString(FormatPreference::kDecimal));
+                           bits_.ToString(FormatPreference::kUnsignedDecimal));
   }
   if (format_ == FormatPreference::kBinary) {
     return absl::StrFormat(

xls/codegen/vast_test.cc

@@ -321,9 +321,9 @@ endmodule)");
 
 TEST_P(VastTest, Literals) {
   VerilogFile f(UseSystemVerilog());
-  EXPECT_EQ("32'd44",
-            f.Literal(UBits(44, 32), SourceInfo(), FormatPreference::kDecimal)
-                ->Emit(nullptr));
+  EXPECT_EQ("32'd44", f.Literal(UBits(44, 32), SourceInfo(),
+                                FormatPreference::kUnsignedDecimal)
+                          ->Emit(nullptr));
   EXPECT_EQ("1'b1",
             f.Literal(UBits(1, 1), SourceInfo(), FormatPreference::kBinary)
                 ->Emit(nullptr));
@@ -344,9 +344,9 @@ TEST_P(VastTest, Literals) {
       Bits huge,
       ParseNumber("0xabcd_000_0000_1234_4321_0000_aaaa_bbbb_cccc_dddd_eeee"));
 
-  EXPECT_EQ(
-      f.Literal(b0, SourceInfo(), FormatPreference::kDecimal)->Emit(nullptr),
-      "1'd0");
+  EXPECT_EQ(f.Literal(b0, SourceInfo(), FormatPreference::kUnsignedDecimal)
+                ->Emit(nullptr),
+            "1'd0");
 
   EXPECT_EQ(f.Literal(b2, SourceInfo(), FormatPreference::kHex)->Emit(nullptr),
             "3'h2");
@@ -365,9 +365,9 @@ TEST_P(VastTest, Literals) {
   EXPECT_EQ(
       f.Literal(b1234, SourceInfo(), FormatPreference::kHex)->Emit(nullptr),
       "55'h00_0000_0000_04d2");
-  EXPECT_EQ(
-      f.Literal(b1234, SourceInfo(), FormatPreference::kDecimal)->Emit(nullptr),
-      "55'd1234");
+  EXPECT_EQ(f.Literal(b1234, SourceInfo(), FormatPreference::kUnsignedDecimal)
+                ->Emit(nullptr),
+            "55'd1234");
   EXPECT_EQ(
       f.Literal(b1234, SourceInfo(), FormatPreference::kBinary)->Emit(nullptr),
       "55'b000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0100_"

xls/dslx/bytecode_interpreter.cc

@@ -969,14 +969,9 @@ absl::Status BytecodeInterpreter::EvalSwap(const Bytecode& bytecode) {
       pieces.push_front(absl::get<std::string>(trace_element));
     } else {
       XLS_RET_CHECK(!stack.empty());
-      // TODO(rspringer): 2022-02-22: This JIT prints values via the IR's
-      // Value::ToHumanString() function. The problem is that it doesn't print
-      // out negative numbers, which is lossy and confusing. Find a way to unify
-      // these two somehow?
       XLS_ASSIGN_OR_RETURN(InterpValue value, Pop(stack));
-      XLS_ASSIGN_OR_RETURN(Value ir_value, value.ConvertToIr());
-      pieces.push_front(
-          ir_value.ToHumanString(absl::get<FormatPreference>(trace_element)));
+      pieces.push_front(value.ToString(
+          /*humanize=*/true, absl::get<FormatPreference>(trace_element)));
     }
   }
 

xls/dslx/interp_value.cc

@@ -108,6 +108,9 @@ static std::string BitsToString(const InterpValue& v, FormatPreference format,
   switch (v.tag()) {
     case InterpValueTag::kUBits: {
       std::string value_str = bits.ToString(format);
+      if (format == FormatPreference::kSignedDecimal && bits.msb()) {
+        value_str = absl::StrCat("-", bits_ops::Negate(bits).ToString(format));
+      }
       if (!include_type_prefix) {
         return value_str;
       }
@@ -116,7 +119,7 @@ static std::string BitsToString(const InterpValue& v, FormatPreference format,
     }
     case InterpValueTag::kSBits: {
       std::string value_str = bits.ToString(format);
-      if ((format == FormatPreference::kDecimal ||
+      if ((format == FormatPreference::kSignedDecimal ||
            format == FormatPreference::kDefault) &&
           bits.msb()) {
         // If we're a signed number in decimal format, give the value for the
@@ -155,8 +158,7 @@ std::string InterpValue::ToString(bool humanize,
     case InterpValueTag::kUBits:
     case InterpValueTag::kSBits:
       return BitsToString(*this, format,
-                          /*include_type_prefix=*/!humanize ||
-                              format == FormatPreference::kBinary);
+                          /*include_type_prefix=*/!humanize);
     case InterpValueTag::kArray:
       return absl::StrFormat("[%s]", make_guts());
     case InterpValueTag::kTuple:

xls/dslx/interp_value_test.cc

@@ -27,18 +27,24 @@ TEST(InterpValueTest, FormatU8) {
   auto ff = InterpValue::MakeUBits(/*bit_count=*/8, /*value=*/0xff);
   EXPECT_EQ(ff.ToString(), "u8:255");
   EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kHex), "0xff");
-  EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kDecimal), "255");
+  EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kSignedDecimal),
+            "-1");
+  EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kUnsignedDecimal),
+            "255");
   EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kBinary),
-            "u8:0b1111_1111");
+            "0b1111_1111");
 }
 
 TEST(InterpValueTest, FormatS8) {
   auto ff = InterpValue::MakeSBits(/*bit_count=*/8, /*value=*/-1);
   EXPECT_EQ(ff.ToString(), "s8:-1");
   EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kHex), "0xff");
-  EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kDecimal), "-1");
+  EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kUnsignedDecimal),
+            "255");
+  EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kSignedDecimal),
+            "-1");
   EXPECT_EQ(ff.ToString(/*humanize=*/true, FormatPreference::kBinary),
-            "s8:0b1111_1111");
+            "0b1111_1111");
 }
 
 TEST(InterpValueTest, BitsEquivalence) {

xls/dslx/interpreter/interpreter_test.py

@@ -205,8 +205,8 @@ def test_tuple_index(self):
 
   def test_for_over_array(self):
     program = textwrap.dedent("""\
-                              #![test]
-                              fn for_over_array_test() {
+    #![test]
+    fn for_over_array_test() {
       let a: u32[3] = u32[3]:[1, 2, 3];
       let result: u32 = for (value, accum): (u32, u32) in a {
         accum + value
@@ -417,23 +417,45 @@ def test_trace(self):
     self.assertIn('4:21-4:32: 1', result.stderr)
     self.assertIn('6:21-6:32: 2', result.stderr)
 
+  def test_trace_s32(self):
+    program = """
+    #![test]
+    fn trace_test() {
+      let x = u32:4;
+      let _ = trace!(x);
+      let x = s32:-1;
+      let _ = trace!(x);
+      ()
+    }
+    """
+    program_file = self.create_tempfile(content=program)
+    # Trace is logged with XLS_LOG(INFO) so log to stderr to capture output.
+    cmd = [
+        _INTERP_PATH, '--compare=none', '--alsologtostderr',
+        program_file.full_path
+    ]
+    result = subp.run(cmd, stderr=subp.PIPE, encoding='utf-8', check=True)
+    self.assertIn('5:21-5:24: 4', result.stderr)
+    self.assertIn('7:21-7:24: -1', result.stderr)
+
   def test_trace_fmt_hello(self):
     program = """
     fn main() {
+      let x = u8:0xF0;
+      let _ = trace_fmt!("Hello world!");
+      let _ = trace_fmt!("x is {}, {:#x} in hex and {:#b} in binary", x, x, x);
       ()
     }
 
     #![test]
     fn hello_test() {
-      let x = u8:0xF0;
-      let _ = trace_fmt!("Hello world!\n");
-      let _ = trace_fmt!("x is {}, {:#x} in hex and {:#b} in binary", x, x, x);
-      ()
+      main()
     }
     """
     program_file = self.create_tempfile(content=program)
     cmd = [_INTERP_PATH, '--alsologtostderr', program_file.full_path]
     result = subp.run(cmd, stderr=subp.PIPE, encoding='utf-8', check=True)
+    print(result.stderr)
     self.assertIn('Hello world!', result.stderr)
     self.assertIn('x is 240, 0xf0 in hex and 0b1111_0000 in binary',
                   result.stderr)

xls/dslx/mangle.cc

@@ -27,8 +27,8 @@ std::string MangleInterpValue(const InterpValue& value) {
   if (value.IsBits() || value.IsEnum()) {
     auto bits_value = InterpValue::MakeBits(/*is_signed=*/value.IsSigned(),
                                             value.GetBitsOrDie());
-    std::string s =
-        bits_value.ToString(/*humanize=*/true, FormatPreference::kDecimal);
+    std::string s = bits_value.ToString(/*humanize=*/true,
+                                        FormatPreference::kUnsignedDecimal);
     // Negatives are not valid characters in IR symbols so replace leading '-'
     // with 'm'.
     return absl::StrReplaceAll(s, {{"-", "m"}});

xls/dslx/run_routines.cc

@@ -139,7 +139,7 @@ absl::Status RunComparator::RunComparison(
   const char* mode_str = nullptr;
   Value ir_result;
   switch (mode_) {
-    case CompareMode::kJit: {
+    case CompareMode::kJit: {  // Compare to IR JIT.
       // TODO(https://github.com/google/xls/issues/506): Also compare events
       // once the DSLX interpreter supports them (and the JIT supports traces).
       XLS_ASSIGN_OR_RETURN(FunctionJit * jit,
@@ -148,7 +148,7 @@ absl::Status RunComparator::RunComparison(
       mode_str = "JIT";
       break;
     }
-    case CompareMode::kInterpreter: {
+    case CompareMode::kInterpreter: {  // Compare to IR interpreter.
       XLS_ASSIGN_OR_RETURN(ir_result, DropInterpreterEvents(InterpretFunction(
                                           ir_function, ir_args)));
       mode_str = "interpreter";

xls/ir/bits.cc

@@ -257,7 +257,7 @@ std::string Bits::ToString(FormatPreference preference,
                            bool include_bit_count) const {
   if (preference == FormatPreference::kDefault) {
     if (bit_count() <= 64) {
-      preference = FormatPreference::kDecimal;
+      preference = FormatPreference::kUnsignedDecimal;
     } else {
       preference = FormatPreference::kHex;
     }
@@ -278,7 +278,21 @@ std::string Bits::ToString(FormatPreference preference,
 std::string Bits::ToRawDigits(FormatPreference preference,
                               bool emit_leading_zeros) const {
   XLS_CHECK_NE(preference, FormatPreference::kDefault);
-  if (preference == FormatPreference::kDecimal) {
+  if (preference == FormatPreference::kSignedDecimal) {
+    // Leading zeros don't make a lot of sense in decimal format as there is no
+    // clean correspondence between decimal digits and binary digits.
+    XLS_CHECK(!emit_leading_zeros)
+        << "emit_leading_zeros not supported for decimal format.";
+
+    // TODO(google/xls#461): 2019-04-03 Add support for arbitrary width decimal
+    // emission.
+    XLS_CHECK(FitsInInt64())
+        << "Decimal output not supported for values which do "
+           "not fit in an int64_t";
+    return absl::StrCat(ToInt64().value());
+  }
+
+  if (preference == FormatPreference::kUnsignedDecimal) {
     // Leading zeros don't make a lot of sense in decimal format as there is no
     // clean correspondence between decimal digits and binary digits.
     XLS_CHECK(!emit_leading_zeros)

xls/ir/bits_test.cc

@@ -317,28 +317,36 @@ TEST(BitsTest, ToString) {
   };
 
   Bits empty_bits(0);
-  EXPECT_EQ(empty_bits.ToString(FormatPreference::kDecimal), "0");
+  EXPECT_EQ(empty_bits.ToString(FormatPreference::kUnsignedDecimal), "0");
+  EXPECT_EQ(empty_bits.ToString(FormatPreference::kSignedDecimal), "0");
   test_hex(empty_bits, "0x0");
   test_binary(empty_bits, "0b0");
-  EXPECT_EQ(empty_bits.ToString(FormatPreference::kDecimal,
+  EXPECT_EQ(empty_bits.ToString(FormatPreference::kUnsignedDecimal,
+                                /*include_bit_count=*/true),
+            "0 [0 bits]");
+  EXPECT_EQ(empty_bits.ToString(FormatPreference::kSignedDecimal,
                                 /*include_bit_count=*/true),
             "0 [0 bits]");
 
   Bits b1 = UBits(1, 1);
-  EXPECT_EQ(b1.ToString(FormatPreference::kDecimal), "1");
+  EXPECT_EQ(b1.ToString(FormatPreference::kUnsignedDecimal), "1");
+  EXPECT_EQ(b1.ToString(FormatPreference::kSignedDecimal), "-1");
   test_hex(b1, "0x1");
   test_binary(b1, "0b1");
 
   test_binary(UBits(1, 16), "0b1");
   test_hex(UBits(1, 16), "0x1");
 
   Bits b42 = UBits(42, 7);
-  EXPECT_EQ(b42.ToString(FormatPreference::kDecimal), "42");
+  EXPECT_EQ(b42.ToString(FormatPreference::kUnsignedDecimal), "42");
+  EXPECT_EQ(b42.ToString(FormatPreference::kSignedDecimal), "42");
   test_hex(b42, "0x2a");
   test_binary(b42, "0b10_1010");
 
   Bits prime64 = PrimeBits(64);
-  EXPECT_EQ(prime64.ToString(FormatPreference::kDecimal),
+  EXPECT_EQ(prime64.ToString(FormatPreference::kUnsignedDecimal),
+            "2892025783495830204");
+  EXPECT_EQ(prime64.ToString(FormatPreference::kSignedDecimal),
             "2892025783495830204");
   test_hex(prime64, "0x2822_8a20_a28a_2abc");
   test_binary(
@@ -363,7 +371,8 @@ TEST(BitsTest, ToString) {
 
 TEST(BitsTest, ToRawString) {
   Bits empty_bits(0);
-  EXPECT_EQ(empty_bits.ToRawDigits(FormatPreference::kDecimal), "0");
+  EXPECT_EQ(empty_bits.ToRawDigits(FormatPreference::kUnsignedDecimal), "0");
+  EXPECT_EQ(empty_bits.ToRawDigits(FormatPreference::kSignedDecimal), "0");
   EXPECT_EQ(empty_bits.ToRawDigits(FormatPreference::kHex), "0");
   EXPECT_EQ(empty_bits.ToRawDigits(FormatPreference::kBinary), "0");
   EXPECT_EQ(empty_bits.ToRawDigits(FormatPreference::kHex,

xls/ir/format_preference.cc

@@ -24,8 +24,10 @@ absl::string_view FormatPreferenceToString(FormatPreference preference) {
       return "default";
     case FormatPreference::kBinary:
       return "binary";
-    case FormatPreference::kDecimal:
-      return "decimal";
+    case FormatPreference::kSignedDecimal:
+      return "signed-decimal";
+    case FormatPreference::kUnsignedDecimal:
+      return "unsigned-decimal";
     case FormatPreference::kHex:
       return "hex";
     case FormatPreference::kPlainBinary:
@@ -43,7 +45,9 @@ absl::string_view FormatPreferenceToXlsSpecifier(FormatPreference preference) {
       return "{}";
     case FormatPreference::kBinary:
       return "{:#b}";
-    case FormatPreference::kDecimal:
+    case FormatPreference::kUnsignedDecimal:
+      return "{:u}";
+    case FormatPreference::kSignedDecimal:
       return "{:d}";
     case FormatPreference::kHex:
       return "{:#x}";
@@ -66,8 +70,10 @@ absl::string_view FormatPreferenceToVerilogSpecifier(
       // - decide that the default format (or at least the format associated
       // with {}) is decimal after all (matches Rust)
       return "%d";
-    case FormatPreference::kDecimal:
+    case FormatPreference::kSignedDecimal:
       return "%d";
+    case FormatPreference::kUnsignedDecimal:
+      return "%u";
     // Technically, the binary and hex format specifications are slightly wrong
     // because Verilog simulators don't break up long values with underscores as
     // XLS does. Practically speaking, though, it isn't worth doing a complex,
@@ -93,8 +99,10 @@ absl::StatusOr<FormatPreference> FormatPreferenceFromString(
     return FormatPreference::kBinary;
   } else if (s == "hex") {
     return FormatPreference::kHex;
-  } else if (s == "decimal") {
-    return FormatPreference::kDecimal;
+  } else if (s == "signed-decimal") {
+    return FormatPreference::kSignedDecimal;
+  } else if (s == "unsigned-decimal") {
+    return FormatPreference::kUnsignedDecimal;
   } else if (s == "plain_binary") {
     return FormatPreference::kPlainBinary;
   } else if (s == "plain_hex") {

xls/ir/format_preference.h

@@ -29,7 +29,8 @@ enum class FormatPreference {
   // hexadecimal is used.
   kDefault,
   kBinary,
-  kDecimal,
+  kSignedDecimal,
+  kUnsignedDecimal,
   kHex,
   kPlainBinary,  // No 0b prefix and no separators as in Rust {:b} and Verilog
                  // %b