Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support for 128 bit constants in dpdk backend #5009

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support for 128 bit constants in dpdk backend #5009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backends/dpdk/DpdkXfail.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ p4c_add_xfail_reason("dpdk"
testdata/p4_16_samples/pna-dpdk-invalid-hdr-warnings5.p4
testdata/p4_16_samples/pna-dpdk-invalid-hdr-warnings6.p4
testdata/p4_16_samples/pna-dpdk-header-union-stack2.p4
testdata/p4_16_samples/dash/dash-pipeline-pna-dpdk.p4
testdata/p4_16_dpdk_errors/psa-dpdk-128bitCast_error.p4
)

p4c_add_xfail_reason("dpdk"
Expand Down
8 changes: 8 additions & 0 deletions backends/dpdk/dpdkArch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2957,6 +2957,14 @@ MoveNonHeaderFieldsToPseudoHeader::addAssignmentStmt(const IR::Expression *e) {
const IR::Node *MoveNonHeaderFieldsToPseudoHeader::postorder(IR::AssignmentStatement *assn) {
if (is_all_args_header) return assn;
auto result = new IR::IndexedVector<IR::StatOrDecl>();
if (isLargeFieldOperand(assn->left) && assn->right->is<IR::Constant>()) {
auto leftType = assn->left->type->to<IR::Type_Bits>()->width_bits();
auto rightType = assn->right->type->to<IR::Type_Bits>()->width_bits();
if (leftType == SupportedBitWidth && rightType == leftType) {
auto cst = assn->right->to<IR::Constant>();
if (!cst->fitsUint64()) return assn;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how constant not fitting in uint64 will be handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[backends/dpdk/dpdkHelpers.cpp] - This file has the changes in this same PR where Unsigned values more than 64 bit is handled.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can the 128 bit case not be handled here? I am a little concerned that we might be masking failures here.

Also: the error message at line 2977 is incorrect and probably similar error messages, too. It looks like the DPDK backend only supports bit widths with power of 2, is that correct?

Copy link
Contributor Author

@Sosutha Sosutha Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code generation for 128 bit constant is handled in dpdkHelpers.cpp since here only the constraints needed for the DPDK is checked in this file. DPDK backend supports till 64 bits and assembly instructions to handle only 128 bit after 64 bits. Hence for the intermediate number of bits, error will be thrown.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure this is working as intended I would example programs that trigger these error cases. Then we can also see what kind of error is thrown and whether that error makes sense.

}
}
if ((isLargeFieldOperand(assn->left) && !isLargeFieldOperand(assn->right) &&
!isInsideHeader(assn->right)) ||
(isLargeFieldOperand(assn->left) && assn->right->is<IR::Constant>())) {
Expand Down
71 changes: 59 additions & 12 deletions backends/dpdk/dpdkHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ bool ConvertStatementToDpdk::preorder(const IR::AssignmentStatement *a) {
"Negate operation is only supported on BIT types");
return false;
}
if (n->expr->type->to<IR::Type_Bits>()->width_bits() == 128) {
if (n->expr->type->to<IR::Type_Bits>()->width_bits() == SupportedBitWidth) {
add128bitComplInstr(left, n->expr);
} else {
BUG_CHECK(metadataStruct, "Metadata structure missing unexpectedly!");
Expand Down Expand Up @@ -632,7 +632,12 @@ bool ConvertStatementToDpdk::preorder(const IR::AssignmentStatement *a) {
}
} else if (right->is<IR::PathExpression>() || right->is<IR::Member>() ||
right->is<IR::BoolLiteral>() || right->is<IR::Constant>()) {
i = new IR::DpdkMovStatement(a->left, a->right);
if (right->is<IR::Constant>() &&
right->type->to<IR::Type_Bits>()->width_bits() == SupportedBitWidth) {
add128bitMovInstr(left, right);
} else {
i = new IR::DpdkMovStatement(left, right);
}
} else {
std::cerr << right->node_type_name() << std::endl;
BUG("Not implemented.");
Expand Down Expand Up @@ -1506,9 +1511,9 @@ bool ConvertStatementToDpdk::preorder(const IR::SwitchStatement *s) {
bool ConvertStatementToDpdk::checkIf128bitOp(const IR::Expression *src1Op,
const IR::Expression *src2Op) {
if (auto src1Type = src1Op->type->to<IR::Type_Bits>()) {
if (src1Type->width_bits() == 128) {
if (src1Type->width_bits() == SupportedBitWidth) {
if (auto src2Type = src2Op->type->to<IR::Type_Bits>()) {
if (src2Type->width_bits() == 128) return true;
if (src2Type->width_bits() == SupportedBitWidth) return true;
}
}
}
Expand Down Expand Up @@ -1559,22 +1564,25 @@ void ConvertStatementToDpdk::add128bitwiseInstr(const IR::Expression *src1Op,
add_instr(new IR::DpdkMovhStatement(src1OpUpper, src1Op));
add_instr(new IR::DpdkMovStatement(src1OpLower, src1Op));
if (src2Op->is<IR::Constant>()) {
auto ConstVal = src2Op->to<IR::Constant>()->value;
auto upperConst = new IR::Constant(ConstVal >> 64);
auto lowerConst = new IR::Constant(ConstVal & 0xFFFFFFFFFFFFFFFF);
add_instr(new IR::DpdkMovStatement(tmp, src1OpLower));
if (strcmp(op, "xor") == 0) {
add_instr(new IR::DpdkXorStatement(tmp, tmp, src2Op));
add_instr(new IR::DpdkXorStatement(tmp, tmp, lowerConst));
} else if (strcmp(op, "or") == 0) {
add_instr(new IR::DpdkOrStatement(tmp, tmp, src2Op));
add_instr(new IR::DpdkOrStatement(tmp, tmp, lowerConst));
} else if (strcmp(op, "and") == 0) {
add_instr(new IR::DpdkAndStatement(tmp, tmp, src2Op));
add_instr(new IR::DpdkAndStatement(tmp, tmp, lowerConst));
}
add_instr(new IR::DpdkMovStatement(src1Op, tmp));
add_instr(new IR::DpdkMovStatement(tmp, src1OpUpper));
if (strcmp(op, "xor") == 0) {
add_instr(new IR::DpdkXorStatement(tmp, tmp, src2Op));
add_instr(new IR::DpdkXorStatement(tmp, tmp, upperConst));
} else if (strcmp(op, "or") == 0) {
add_instr(new IR::DpdkOrStatement(tmp, tmp, src2Op));
add_instr(new IR::DpdkOrStatement(tmp, tmp, upperConst));
} else if (strcmp(op, "and") == 0) {
add_instr(new IR::DpdkAndStatement(tmp, tmp, src2Op));
add_instr(new IR::DpdkAndStatement(tmp, tmp, upperConst));
}
add_instr(new IR::DpdkMovhStatement(src1Op, tmp));
} else {
Expand Down Expand Up @@ -1659,8 +1667,11 @@ void ConvertStatementToDpdk::add128ComparisonInstr(cstring true_label, const IR:
add_instr(new IR::DpdkMovhStatement(src1OpUpper, src1Op));
add_instr(new IR::DpdkMovStatement(src1OpLower, src1Op));
if (src2Op->is<IR::Constant>()) {
add_instr(new IR::DpdkXorStatement(src1OpUpper, src1OpUpper, src2Op));
add_instr(new IR::DpdkXorStatement(src1OpLower, src1OpLower, src2Op));
auto ConstVal = src2Op->to<IR::Constant>()->value;
auto upperConst = new IR::Constant(ConstVal >> 64);
auto lowerConst = new IR::Constant(ConstVal & 0xFFFFFFFFFFFFFFFF);
add_instr(new IR::DpdkXorStatement(src1OpUpper, src1OpUpper, upperConst));
add_instr(new IR::DpdkXorStatement(src1OpLower, src1OpLower, lowerConst));
} else {
auto src2OpHeaderName = src2Op->toString();
if (src2Op->is<IR::Member>()) {
Expand Down Expand Up @@ -1734,4 +1745,40 @@ void ConvertStatementToDpdk::add128bitComplInstr(const IR::Expression *dst,
add_instr(new IR::DpdkMovStatement(dst, src1OpLower));
add_instr(new IR::DpdkMovhStatement(dst, src1OpUpper));
}

void ConvertStatementToDpdk::add128bitMovInstr(const IR::Expression *left,
const IR::Expression *right) {
if (!createSandboxHeaderType) {
createSandboxHeaderType = true;
createSandboxHeader();
}
const IR::Type_Header *Type_Header = nullptr;
for (auto header : structure->header_types) {
if (header.first == "_p4c_sandbox_header_t") {
Type_Header = header.second;
}
}
if (Type_Header == nullptr) {
BUG("Header type not found");
}
auto leftHeaderName = left->toString();
if (left->is<IR::Member>()) {
leftHeaderName = left->to<IR::Member>()->member.name;
}
leftHeaderName = leftHeaderName + "_128";
auto leftHeader = new IR::Declaration_Variable(leftHeaderName, Type_Header);
auto leftHeaderInstance = new IR::DpdkHeaderInstance(leftHeader, Type_Header);
structure->addHeaderInstances(leftHeaderInstance);
auto leftUpper =
new IR::Member(new IR::PathExpression("h." + leftHeader->name), IR::ID("upper_half"));
auto leftLower =
new IR::Member(new IR::PathExpression("h." + leftHeader->name), IR::ID("lower_half"));
auto ConstVal = right->to<IR::Constant>()->value;
auto upperConst = new IR::Constant(ConstVal >> 64);
auto lowerConst = new IR::Constant(ConstVal & 0xFFFFFFFFFFFFFFFF);
add_instr(new IR::DpdkMovStatement(leftUpper, upperConst));
add_instr(new IR::DpdkMovStatement(leftLower, lowerConst));
add_instr(new IR::DpdkMovStatement(left, leftLower));
add_instr(new IR::DpdkMovhStatement(left, leftUpper));
}
} // namespace P4::DPDK
3 changes: 3 additions & 0 deletions backends/dpdk/dpdkHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ limitations under the License.

namespace P4::DPDK {

static const int SupportedBitWidth = 128;

class ConvertStatementToDpdk;

/// @brief Name of the metadata used as output port.
Expand Down Expand Up @@ -199,6 +201,7 @@ class ConvertStatementToDpdk : public Inspector {
void add128ComparisonInstr(cstring true_label, const IR::Expression *src1Op,
const IR::Expression *src2Op, const char *op);
void add128bitComplInstr(const IR::Expression *, const IR::Expression *);
void add128bitMovInstr(const IR::Expression *left, const IR::Expression *right);
};
/// Only simplify complex expression in ingress/egress.
class ProcessControls : public P4::RemoveComplexExpressionsPolicy {
Expand Down
Loading
Loading