diff --git a/xls/codegen/vast/dslx_builder.cc b/xls/codegen/vast/dslx_builder.cc index c1f500fa8c..2a5ab63154 100644 --- a/xls/codegen/vast/dslx_builder.cc +++ b/xls/codegen/vast/dslx_builder.cc @@ -439,13 +439,16 @@ absl::StatusOr DslxBuilder::ConvertMaxToWidth( dslx::Number* one = module().Make( span, "1", dslx::NumberKind::kOther, type_annot); return module().Make(span, dslx::BinopKind::kAdd, casted_max, - one); + one, span); } dslx::Unop* DslxBuilder::HandleUnaryOperator(const dslx::Span& span, dslx::UnopKind unop_kind, dslx::Expr* arg) { - return module().Make(span, dslx::UnopKind::kNegate, arg); + // Note it uses the same span for the whole node and the operand; + // the only time the operand span is used is for formatting, and + // this node won't be used for formatting. + return module().Make(span, dslx::UnopKind::kNegate, arg, span); } absl::StatusOr DslxBuilder::HandleIntegerExponentiation( diff --git a/xls/codegen/vast/translate_vast_to_dslx.cc b/xls/codegen/vast/translate_vast_to_dslx.cc index e58ba49f68..1f994937e1 100644 --- a/xls/codegen/vast/translate_vast_to_dslx.cc +++ b/xls/codegen/vast/translate_vast_to_dslx.cc @@ -213,7 +213,10 @@ class VastToDslxTranslator { "%s vs %s", op->Emit(nullptr), lhs_type->ToString(), rhs_type->ToString())); } - result = module().Make(span, kind, lhs, rhs); + // Note it uses the same span for the whole node and the operand; + // the only time the operand span is used is for formatting, and + // this node won't be used for formatting. + result = module().Make(span, kind, lhs, rhs, span); } return dslx_builder_->MaybeCastToInferredVastType(op, result); } @@ -266,11 +269,14 @@ class VastToDslxTranslator { dslx_expr = module().Make(dslx_expr->span(), dslx_expr, annot); } + dslx::Span span = CreateNodeSpan(concat); + // Note it uses the same span for the whole node and the operand; + // the only time the operand span is used is for formatting, and + // this node won't be used for formatting. result = result == nullptr ? dslx_expr - : module().Make(CreateNodeSpan(concat), - dslx::BinopKind::kConcat, - result, dslx_expr); + : module().Make(span, dslx::BinopKind::kConcat, + result, dslx_expr, span); } return result; } diff --git a/xls/dslx/frontend/ast.cc b/xls/dslx/frontend/ast.cc index 9a4c949aa1..592b2abad3 100644 --- a/xls/dslx/frontend/ast.cc +++ b/xls/dslx/frontend/ast.cc @@ -1612,9 +1612,10 @@ absl::StatusOr BinopKindFromString(std::string_view s) { } Binop::Binop(Module* owner, Span span, BinopKind binop_kind, Expr* lhs, - Expr* rhs, bool in_parens) + Expr* rhs, Span op_span, bool in_parens) : Expr(owner, std::move(span), in_parens), binop_kind_(binop_kind), + op_span_(op_span), lhs_(lhs), rhs_(rhs) {} diff --git a/xls/dslx/frontend/ast.h b/xls/dslx/frontend/ast.h index 419c3aad45..250851864b 100644 --- a/xls/dslx/frontend/ast.h +++ b/xls/dslx/frontend/ast.h @@ -1388,9 +1388,10 @@ std::string UnopKindToString(UnopKind k); class Unop : public Expr { public: Unop(Module* owner, Span span, UnopKind unop_kind, Expr* operand, - bool in_parens = false) + Span op_span, bool in_parens = false) : Expr(owner, std::move(span), in_parens), unop_kind_(unop_kind), + op_span_(op_span), operand_(operand) {} ~Unop() override; @@ -1412,6 +1413,8 @@ class Unop : public Expr { UnopKind unop_kind() const { return unop_kind_; } Expr* operand() const { return operand_; } + // The span of the operator, e.g., `++` for `x ++ y`. + Span op_span() const { return op_span_; } Precedence GetPrecedenceWithoutParens() const final { return Precedence::kUnaryOp; @@ -1422,6 +1425,7 @@ class Unop : public Expr { UnopKind unop_kind_; Expr* operand_; + Span op_span_; }; #define XLS_DSLX_BINOP_KIND_EACH(X) \ @@ -1484,7 +1488,7 @@ const absl::btree_set& GetBinopShifts(); class Binop : public Expr { public: Binop(Module* owner, Span span, BinopKind kind, Expr* lhs, Expr* rhs, - bool in_parens = false); + Span op_span, bool in_parens = false); ~Binop() override; @@ -1505,6 +1509,8 @@ class Binop : public Expr { BinopKind binop_kind() const { return binop_kind_; } Expr* lhs() const { return lhs_; } Expr* rhs() const { return rhs_; } + // The span of the operator, e.g., `++` for `x ++ y`. + Span op_span() const { return op_span_; } Precedence GetPrecedenceWithoutParens() const final; @@ -1512,6 +1518,7 @@ class Binop : public Expr { std::string ToStringInternal() const final; BinopKind binop_kind_; + Span op_span_; Expr* lhs_; Expr* rhs_; }; diff --git a/xls/dslx/frontend/ast_cloner.cc b/xls/dslx/frontend/ast_cloner.cc index a24536e691..1377984abb 100644 --- a/xls/dslx/frontend/ast_cloner.cc +++ b/xls/dslx/frontend/ast_cloner.cc @@ -99,7 +99,8 @@ class AstCloner : public AstNodeVisitor { XLS_RETURN_IF_ERROR(VisitChildren(n)); old_to_new_[n] = module_->Make( n->span(), n->binop_kind(), down_cast(old_to_new_.at(n->lhs())), - down_cast(old_to_new_.at(n->rhs())), n->in_parens()); + down_cast(old_to_new_.at(n->rhs())), n->op_span(), + n->in_parens()); return absl::OkStatus(); } @@ -926,9 +927,10 @@ class AstCloner : public AstNodeVisitor { absl::Status HandleUnop(const Unop* n) override { XLS_RETURN_IF_ERROR(VisitChildren(n)); - old_to_new_[n] = module_->Make( - n->span(), n->unop_kind(), - down_cast(old_to_new_.at(n->operand())), n->in_parens()); + old_to_new_[n] = + module_->Make(n->span(), n->unop_kind(), + down_cast(old_to_new_.at(n->operand())), + n->op_span(), n->in_parens()); return absl::OkStatus(); } diff --git a/xls/dslx/frontend/ast_test.cc b/xls/dslx/frontend/ast_test.cc index 2676518be9..7b5b21ad7a 100644 --- a/xls/dslx/frontend/ast_test.cc +++ b/xls/dslx/frontend/ast_test.cc @@ -262,7 +262,8 @@ TEST(AstTest, IsConstantBinopOfNameRefs) { m.Make(fake_span, std::string("MyStruct"), nullptr); NameRef* left = m.Make(fake_span, "name", name_def); NameRef* right = m.Make(fake_span, "name", name_def); - Binop* binop = m.Make(fake_span, BinopKind::kAdd, left, right); + Binop* binop = + m.Make(fake_span, BinopKind::kAdd, left, right, Span::Fake()); EXPECT_FALSE(IsConstant(binop)); } @@ -276,7 +277,8 @@ TEST(AstTest, IsConstantBinopOfNumbers) { NumberKind::kOther, /*type=*/nullptr); Number* right = m.Make(fake_span, std::string("42"), NumberKind::kOther, /*type=*/nullptr); - Binop* binop = m.Make(fake_span, BinopKind::kAdd, left, right); + Binop* binop = + m.Make(fake_span, BinopKind::kAdd, left, right, Span::Fake()); EXPECT_TRUE(IsConstant(binop)); } @@ -289,7 +291,7 @@ TEST(AstTest, IsConstantUnopOfNameRefs) { NameDef* name_def = m.Make(fake_span, std::string("MyStruct"), nullptr); NameRef* operand = m.Make(fake_span, "name", name_def); - Unop* unop = m.Make(fake_span, UnopKind::kNegate, operand); + Unop* unop = m.Make(fake_span, UnopKind::kNegate, operand, fake_span); EXPECT_FALSE(IsConstant(unop)); } @@ -301,7 +303,7 @@ TEST(AstTest, IsConstantUnopOfNumbers) { Number* operand = m.Make(fake_span, std::string("41"), NumberKind::kOther, /*type=*/nullptr); - Unop* unop = m.Make(fake_span, UnopKind::kNegate, operand); + Unop* unop = m.Make(fake_span, UnopKind::kNegate, operand, fake_span); EXPECT_TRUE(IsConstant(unop)); } diff --git a/xls/dslx/frontend/ast_test_utils.cc b/xls/dslx/frontend/ast_test_utils.cc index f750015408..2825bbb414 100644 --- a/xls/dslx/frontend/ast_test_utils.cc +++ b/xls/dslx/frontend/ast_test_utils.cc @@ -54,7 +54,7 @@ std::tuple MakeCastWithinLtComparison() { // x as t < x Cast* cast = m.Make(fake_span, x_ref, type_ref_type_annotation); - Binop* lt = m.Make(fake_span, BinopKind::kLt, cast, x_ref); + Binop* lt = m.Make(fake_span, BinopKind::kLt, cast, x_ref, fake_span); return std::make_tuple(file_table, std::move(m), lt); } @@ -134,7 +134,7 @@ std::tuple MakeCastWithinNegateExpression() { // x as u32 Cast* cast = m.Make(fake_span, x_ref, builtin_u32); - Unop* unop = m.Make(fake_span, UnopKind::kNegate, cast); + Unop* unop = m.Make(fake_span, UnopKind::kNegate, cast, fake_span); return std::make_tuple(file_table, std::move(m), unop); } @@ -147,7 +147,8 @@ std::tuple MakeArithWithinAttrExpression() { BuiltinNameDef* y_def = m.GetOrCreateBuiltinNameDef("y"); NameRef* y_ref = m.Make(fake_span, "y", y_def); - Binop* binop = m.Make(fake_span, BinopKind::kMul, x_ref, y_ref); + Binop* binop = + m.Make(fake_span, BinopKind::kMul, x_ref, y_ref, fake_span); Attr* attr = m.Make(fake_span, binop, "my_attr"); return std::make_tuple(file_table, std::move(m), attr); } diff --git a/xls/dslx/frontend/parser.cc b/xls/dslx/frontend/parser.cc index 87339156dc..ad076f7ee0 100644 --- a/xls/dslx/frontend/parser.cc +++ b/xls/dslx/frontend/parser.cc @@ -1305,7 +1305,7 @@ absl::StatusOr Parser::ParseBinopChain( XLS_ASSIGN_OR_RETURN(BinopKind kind, BinopKindFromString(TokenKindToString(op.kind()))); Span span(lhs->span().start(), rhs->span().limit()); - lhs = module_->Make(span, kind, lhs, rhs); + lhs = module_->Make(span, kind, lhs, rhs, op.span()); } else { break; } @@ -1413,7 +1413,7 @@ absl::StatusOr Parser::ParseComparisonExpression( "comparison operators cannot be chained"); } Span span(lhs->span().start(), rhs->span().limit()); - lhs = module_->Make(span, kind, lhs, rhs); + lhs = module_->Make(span, kind, lhs, rhs, op.span()); } VLOG(5) << "ParseComparisonExpression; result: `" << lhs->ToString() << "`"; return lhs; @@ -1781,7 +1781,7 @@ absl::StatusOr Parser::ParseTermLhs(Bindings& outer_bindings, LOG(FATAL) << "Inconsistent unary operation token kind."; } Span span(tok.span().start(), arg->span().limit()); - lhs = module_->Make(span, unop_kind, arg); + lhs = module_->Make(span, unop_kind, arg, tok.span()); } else if (peek->IsTypeKeyword() || (peek->kind() == TokenKind::kIdentifier && outer_bindings.ResolveNodeIsTypeDefinition(*peek->GetValue()))) { diff --git a/xls/dslx/type_system/type_test.cc b/xls/dslx/type_system/type_test.cc index 926c14d276..0c761ea95f 100644 --- a/xls/dslx/type_system/type_test.cc +++ b/xls/dslx/type_system/type_test.cc @@ -162,7 +162,7 @@ StructType CreateStructWithParametricExpr(Module& module) { NameDef* type_n_name_def = module.Make( Span::Fake(), "N", module.Make(Span::Fake(), BinopKind::kAdd, type_m_name_ref, - type_m_name_ref)); + type_m_name_ref, Span::Fake())); bindings.push_back( module.Make(type_n_name_def, u32_type_annot, nullptr)); diff --git a/xls/dslx/type_system_v2/inference_table_test.cc b/xls/dslx/type_system_v2/inference_table_test.cc index c9a7ee0047..7af9842ad6 100644 --- a/xls/dslx/type_system_v2/inference_table_test.cc +++ b/xls/dslx/type_system_v2/inference_table_test.cc @@ -159,8 +159,8 @@ TEST_F(InferenceTableTest, SignednessMismatch) { NameDef* y = module_->Make(Span::Fake(), "y", /*definer=*/nullptr); NameRef* x_ref = module_->Make(Span::Fake(), "x", x); NameRef* y_ref = module_->Make(Span::Fake(), "y", y); - AstNode* add_node = - module_->Make(Span::Fake(), BinopKind::kAdd, x_ref, y_ref); + AstNode* add_node = module_->Make(Span::Fake(), BinopKind::kAdd, x_ref, + y_ref, Span::Fake()); TypeAnnotation* u32_annotation = module_->Make( Span::Fake(), BuiltinType::kU32, @@ -190,8 +190,8 @@ TEST_F(InferenceTableTest, SignednessAgreement) { NameDef* y = module_->Make(Span::Fake(), "y", /*definer=*/nullptr); NameRef* x_ref = module_->Make(Span::Fake(), "x", x); NameRef* y_ref = module_->Make(Span::Fake(), "y", y); - AstNode* add_node = - module_->Make(Span::Fake(), BinopKind::kAdd, x_ref, y_ref); + AstNode* add_node = module_->Make(Span::Fake(), BinopKind::kAdd, x_ref, + y_ref, Span::Fake()); TypeAnnotation* u32_annotation = module_->Make( Span::Fake(), BuiltinType::kU32, diff --git a/xls/fuzzer/ast_generator.cc b/xls/fuzzer/ast_generator.cc index 2fd6305b07..1b79268172 100644 --- a/xls/fuzzer/ast_generator.cc +++ b/xls/fuzzer/ast_generator.cc @@ -353,7 +353,8 @@ absl::StatusOr AstGenerator::GenerateCompare(Context* ctx) { XLS_ASSIGN_OR_RETURN(auto pair, ChooseEnvValueBitsPair(&ctx->env)); TypedExpr lhs = pair.first; TypedExpr rhs = pair.second; - Binop* binop = module_->Make(fake_span_, op, lhs.expr, rhs.expr); + Binop* binop = + module_->Make(fake_span_, op, lhs.expr, rhs.expr, fake_span_); return TypedExpr{.expr = binop, .type = MakeTypeAnnotation(false, 1), .last_delaying_op = ComposeDelayingOps(lhs.last_delaying_op, @@ -626,12 +627,12 @@ absl::StatusOr AstGenerator::GenerateCompareArray(Context* ctx) { XLS_ASSIGN_OR_RETURN(TypedExpr lhs, ChooseEnvValueArray(&ctx->env)); XLS_ASSIGN_OR_RETURN(TypedExpr rhs, ChooseEnvValue(&ctx->env, lhs.type)); BinopKind op = RandomBool(0.5) ? BinopKind::kEq : BinopKind::kNe; - return TypedExpr{ - .expr = module_->Make(fake_span_, op, lhs.expr, rhs.expr), - .type = MakeTypeAnnotation(false, 1), - .last_delaying_op = - ComposeDelayingOps(lhs.last_delaying_op, rhs.last_delaying_op), - .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; + return TypedExpr{.expr = module_->Make(fake_span_, op, lhs.expr, + rhs.expr, fake_span_), + .type = MakeTypeAnnotation(false, 1), + .last_delaying_op = ComposeDelayingOps(lhs.last_delaying_op, + rhs.last_delaying_op), + .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; } class FindTokenTypeVisitor : public AstNodeVisitorWithDefault { @@ -783,12 +784,12 @@ absl::StatusOr AstGenerator::GenerateCompareTuple(Context* ctx) { ChooseEnvValueTupleWithoutToken(&ctx->env)); XLS_ASSIGN_OR_RETURN(TypedExpr rhs, ChooseEnvValue(&ctx->env, lhs.type)); BinopKind op = RandomBool(0.5) ? BinopKind::kEq : BinopKind::kNe; - return TypedExpr{ - .expr = module_->Make(fake_span_, op, lhs.expr, rhs.expr), - .type = MakeTypeAnnotation(false, 1), - .last_delaying_op = - ComposeDelayingOps(lhs.last_delaying_op, rhs.last_delaying_op), - .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; + return TypedExpr{.expr = module_->Make(fake_span_, op, lhs.expr, + rhs.expr, fake_span_), + .type = MakeTypeAnnotation(false, 1), + .last_delaying_op = ComposeDelayingOps(lhs.last_delaying_op, + rhs.last_delaying_op), + .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; } absl::StatusOr AstGenerator::GenerateExprOfType( @@ -1059,11 +1060,12 @@ absl::StatusOr AstGenerator::GenerateSynthesizableDiv(Context* ctx) { XLS_ASSIGN_OR_RETURN(int64_t bit_count, BitsTypeGetBitCount(lhs.type)); Bits divisor = GenerateBits(bit_gen_, bit_count); Number* divisor_node = GenerateNumberFromBits(divisor, lhs.type); - return TypedExpr{.expr = module_->Make(fake_span_, BinopKind::kDiv, - lhs.expr, divisor_node), - .type = lhs.type, - .last_delaying_op = lhs.last_delaying_op, - .min_stage = lhs.min_stage}; + return TypedExpr{ + .expr = module_->Make(fake_span_, BinopKind::kDiv, lhs.expr, + divisor_node, fake_span_), + .type = lhs.type, + .last_delaying_op = lhs.last_delaying_op, + .min_stage = lhs.min_stage}; } absl::StatusOr AstGenerator::GenerateShift(Context* ctx) { @@ -1094,12 +1096,12 @@ absl::StatusOr AstGenerator::GenerateShift(Context* ctx) { rhs.expr = MakeNumber(shift_amount); } } - return TypedExpr{ - .expr = module_->Make(fake_span_, op, lhs.expr, rhs.expr), - .type = lhs.type, - .last_delaying_op = - ComposeDelayingOps(lhs.last_delaying_op, rhs.last_delaying_op), - .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; + return TypedExpr{.expr = module_->Make(fake_span_, op, lhs.expr, + rhs.expr, fake_span_), + .type = lhs.type, + .last_delaying_op = ComposeDelayingOps(lhs.last_delaying_op, + rhs.last_delaying_op), + .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; } absl::StatusOr @@ -1154,8 +1156,8 @@ AstGenerator::GeneratePartialProductDeterministicGroup(Context* ctx) { /*index=*/MakeNumber(0)); auto mulp_rhs = module_->Make(fake_span_, mulp_name_ref, /*index=*/MakeNumber(1)); - Expr* sum = - module_->Make(fake_span_, BinopKind::kAdd, mulp_lhs, mulp_rhs); + Expr* sum = module_->Make(fake_span_, BinopKind::kAdd, mulp_lhs, + mulp_rhs, fake_span_); if (is_signed) { // For smul we have to cast the summation to signed. sum = module_->Make(fake_span_, sum, signed_type); } @@ -1186,12 +1188,12 @@ absl::StatusOr AstGenerator::GenerateBinop(Context* ctx) { if (GetBinopShifts().contains(op)) { return GenerateShift(ctx); } - return TypedExpr{ - .expr = module_->Make(fake_span_, op, lhs.expr, rhs.expr), - .type = lhs.type, - .last_delaying_op = - ComposeDelayingOps(lhs.last_delaying_op, rhs.last_delaying_op), - .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; + return TypedExpr{.expr = module_->Make(fake_span_, op, lhs.expr, + rhs.expr, fake_span_), + .type = lhs.type, + .last_delaying_op = ComposeDelayingOps(lhs.last_delaying_op, + rhs.last_delaying_op), + .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; } absl::StatusOr AstGenerator::GenerateLogicalOp(Context* ctx) { @@ -1204,12 +1206,12 @@ absl::StatusOr AstGenerator::GenerateLogicalOp(Context* ctx) { BinopKind op = RandomChoice( absl::MakeConstSpan({BinopKind::kAnd, BinopKind::kOr, BinopKind::kXor}), bit_gen_); - return TypedExpr{ - .expr = module_->Make(fake_span_, op, lhs.expr, rhs.expr), - .type = lhs.type, - .last_delaying_op = - ComposeDelayingOps(lhs.last_delaying_op, rhs.last_delaying_op), - .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; + return TypedExpr{.expr = module_->Make(fake_span_, op, lhs.expr, + rhs.expr, fake_span_), + .type = lhs.type, + .last_delaying_op = ComposeDelayingOps(lhs.last_delaying_op, + rhs.last_delaying_op), + .min_stage = std::max(lhs.min_stage, rhs.min_stage)}; } Number* AstGenerator::MakeNumber(int64_t value, TypeAnnotation* type) { @@ -1496,8 +1498,8 @@ absl::StatusOr AstGenerator::GenerateArrayConcat(Context* ctx) { auto* rhs_array_type = dynamic_cast(rhs.type); XLS_RET_CHECK(rhs_array_type != nullptr); - Binop* result = - module_->Make(fake_span_, BinopKind::kConcat, lhs.expr, rhs.expr); + Binop* result = module_->Make(fake_span_, BinopKind::kConcat, lhs.expr, + rhs.expr, fake_span_); int64_t result_size = GetArraySize(lhs_array_type) + GetArraySize(rhs_array_type); Number* dim = MakeNumber(result_size); @@ -1677,7 +1679,7 @@ absl::StatusOr AstGenerator::GenerateConcat(Context* ctx) { Expr* result = e.expr; for (int64_t i = 1; i < count; ++i) { result = module_->Make(fake_span_, BinopKind::kConcat, result, - operands[i].expr); + operands[i].expr, fake_span_); } TypeAnnotation* return_type = MakeTypeAnnotation(false, total_width); @@ -2096,7 +2098,7 @@ absl::StatusOr AstGenerator::GenerateUnop(Context* ctx) { UnopKind op = RandomChoice( absl::MakeConstSpan({UnopKind::kInvert, UnopKind::kNegate}), bit_gen_); return TypedExpr{ - .expr = module_->Make(fake_span_, op, arg.expr), + .expr = module_->Make(fake_span_, op, arg.expr, fake_span_), .type = arg.type, .last_delaying_op = arg.last_delaying_op, .min_stage = arg.min_stage, diff --git a/xls/fuzzer/ast_generator.h b/xls/fuzzer/ast_generator.h index 8daa705f2e..9af1bbb052 100644 --- a/xls/fuzzer/ast_generator.h +++ b/xls/fuzzer/ast_generator.h @@ -596,7 +596,8 @@ class AstGenerator { return module_->Make(fake_span_, test, consequent, alternate); } Expr* MakeGe(Expr* lhs, Expr* rhs) { - return module_->Make(fake_span_, BinopKind::kGe, lhs, rhs); + return module_->Make(fake_span_, BinopKind::kGe, lhs, rhs, + fake_span_); } // Creates a number AST node with value 'value' represented in a decimal