Skip to content

Commit

Permalink
[DSLX:TS] Properly type check signed number concat as an error.
Browse files Browse the repository at this point in the history
Fixes #1265

PiperOrigin-RevId: 599977690
  • Loading branch information
cdleary authored and copybara-github committed Jan 20, 2024
1 parent ad567ac commit 697061d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 10 deletions.
14 changes: 10 additions & 4 deletions docs_src/dslx_reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -1260,8 +1260,11 @@ operations return a result of type `bits[1]`, aka `bool`.

Bitwise concatenation is performed with the `++` operator. The value on the left
hand side becomes the most significant bits, the value on the right hand side
becomes the least significant bits. These may be chained together as shown
below:
becomes the least significant bits. Both of the operands must be unsigned (see
[numerical conversions](#numerical-conversions) for details on converting signed
numbers to unsigned).

Concatenation operations may be chained together as shown:

```dslx
#[test]
Expand Down Expand Up @@ -1603,14 +1606,17 @@ for semantics of numeric casts:
fn test_numerical_conversions() {
let s8_m2 = s8:-2;
let u8_m2 = u8:0xfe;
// Sign extension (source type is signed).
// Sign extension (source type is signed, and we widen it).
assert_eq(s32:-2, s8_m2 as s32);
assert_eq(u32:0xfffffffe, s8_m2 as u32);
assert_eq(s16:-2, s8_m2 as s16);
assert_eq(u16:0xfffe, s8_m2 as u16);
// Zero extension (source type is unsigned).
// Zero extension (source type is unsigned, and we widen it).
assert_eq(u32:0xfe, u8_m2 as u32);
assert_eq(s32:0xfe, u8_m2 as s32);
// Nop (bitwidth is unchanged).
assert_eq(s8:-2, s8_m2 as s8);
assert_eq(s8:-2, u8_m2 as s8);
Expand Down
7 changes: 7 additions & 0 deletions xls/dslx/type_system/deduce_expr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,13 @@ static absl::StatusOr<std::unique_ptr<ConcreteType>> DeduceConcat(
"either both-arrays or both-bits");
}

if (lhs_bits->is_signed() || rhs_bits->is_signed()) {
return ctx->TypeMismatchError(
node->span(), node->lhs(), *lhs, node->rhs(), *rhs,
"Concatenation requires operand types to both be "
"unsigned bits");
}

XLS_RET_CHECK(lhs_bits != nullptr);
XLS_RET_CHECK(rhs_bits != nullptr);
XLS_ASSIGN_OR_RETURN(ConcreteTypeDim new_size,
Expand Down
22 changes: 16 additions & 6 deletions xls/dslx/type_system/typecheck_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2097,7 +2097,6 @@ fn f(x: u32) -> u32 {
)";
XLS_ASSERT_OK_AND_ASSIGN(TypecheckedModule tm, Typecheck(program));
ASSERT_THAT(tm.warnings.warnings().size(), 1);
std::string filename = "fake.x";
EXPECT_EQ(tm.warnings.warnings().at(0).message,
"Definition of `y` (type `uN[32]`) is not used in function `f`");
}
Expand All @@ -2111,7 +2110,6 @@ fn f(t: (u32, u32, u32, u32, u32)) -> u32 {
)";
XLS_ASSERT_OK_AND_ASSIGN(TypecheckedModule tm, Typecheck(program));
ASSERT_THAT(tm.warnings.warnings().size(), 5);
std::string filename = "fake.x";
EXPECT_EQ(tm.warnings.warnings().at(0).message,
"Definition of `a` (type `uN[32]`) is not used in function `f`");
EXPECT_EQ(tm.warnings.warnings().at(1).message,
Expand All @@ -2134,7 +2132,6 @@ fn f(x: u32) -> u32 {
)";
XLS_ASSERT_OK_AND_ASSIGN(TypecheckedModule tm, Typecheck(program));
ASSERT_THAT(tm.warnings.warnings().size(), 1);
std::string filename = "fake.x";
EXPECT_EQ(tm.warnings.warnings().at(0).message,
"Definition of `y` (type `uN[32]`) is not used in function `f`");
}
Expand All @@ -2143,12 +2140,25 @@ TEST(TypecheckTest, ConcatU1U1) {
XLS_ASSERT_OK(Typecheck("fn f(x: u1, y: u1) -> u2 { x ++ y }"));
}

TEST(TypecheckTest, ConcatU1S1) {
XLS_ASSERT_OK(Typecheck("fn f(x: u1, y: s1) -> u2 { x ++ y }"));
TEST(TypecheckErrorTest, ConcatU1S1) {
EXPECT_THAT(
Typecheck("fn f(x: u1, y: s1) -> u2 { x ++ y }").status(),
IsPosError("XlsTypeError", HasSubstr("Concatenation requires operand "
"types to both be unsigned bits")));
}

TEST(TypecheckErrorTest, ConcatS1S1) {
EXPECT_THAT(
Typecheck("fn f(x: s1, y: s1) -> u2 { x ++ y }").status(),
IsPosError("XlsTypeError", HasSubstr("Concatenation requires operand "
"types to both be unsigned bits")));
}

TEST(TypecheckTest, ConcatU2S1) {
XLS_ASSERT_OK(Typecheck("fn f(x: u2, y: s1) -> u3 { x ++ y }"));
EXPECT_THAT(
Typecheck("fn f(x: u2, y: s1) -> u3 { x ++ y }").status(),
IsPosError("XlsTypeError", HasSubstr("Concatenation requires operand "
"types to both be unsigned bits")));
}

TEST(TypecheckTest, ConcatU1Nil) {
Expand Down

0 comments on commit 697061d

Please sign in to comment.