Skip to content

Commit

Permalink
P4C-DPDK - Incorrect constant value when there is a substract operati…
Browse files Browse the repository at this point in the history
…on involved with a constant
  • Loading branch information
github-sajan committed Jun 28, 2022
1 parent 45014a7 commit 40cb5a0
Show file tree
Hide file tree
Showing 25 changed files with 815 additions and 8 deletions.
48 changes: 40 additions & 8 deletions backends/dpdk/dpdkHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ bool ConvertStatementToDpdk::preorder(const IR::AssignmentStatement *a) {
} else if (auto r = right->to<IR::Operation_Binary>()) {
auto src1Op = r->left;
auto src2Op = r->right;
bool isSignExtended = false;
if (r->left->is<IR::Constant>()) {
if (isCommutativeBinaryOperation(r)) {
src1Op = r->right;
Expand All @@ -134,27 +135,58 @@ bool ConvertStatementToDpdk::preorder(const IR::AssignmentStatement *a) {
src1Op = src1Member;
}
}
/* If a constant is present in binary operation and it is not 8-bit
aligned and it is signed, then the constant value is sign extended
with a width of either 32 bit if its original bit-width is less than
32 bit or 64 bit if the bit-width of constant is greater than 32 bit
*/

if (!isEightBitAligned(src2Op) && src2Op->is<IR::Constant>()) {
if (auto tb = src2Op->type->to<IR::Type_Bits>()) {
if (tb != nullptr && tb->isSigned) {
isSignExtended = true;
auto consOrgBitwidth = src2Op->to<IR::Constant>()->type->width_bits();
auto bitWidth = consOrgBitwidth < 32 ? 32 : 64;
auto consValue = src2Op->to<IR::Constant>()->value;
auto val = consValue << (bitWidth - consOrgBitwidth);
consValue = val >> (bitWidth - consOrgBitwidth);
src2Op = new IR::Constant(IR::Type_Bits::get(bitWidth), consValue);
}
}
}

if (right->is<IR::Add>()) {
i = new IR::DpdkAddStatement(left, src1Op, src2Op);
add_instr(new IR::DpdkAddStatement(left, src1Op, src2Op));
} else if (right->is<IR::Sub>()) {
i = new IR::DpdkSubStatement(left, src1Op, src2Op);
add_instr(new IR::DpdkSubStatement(left, src1Op, src2Op));
} else if (right->is<IR::Shl>()) {
i = new IR::DpdkShlStatement(left, src1Op, src2Op);
add_instr(new IR::DpdkShlStatement(left, src1Op, src2Op));
} else if (right->is<IR::Shr>()) {
i = new IR::DpdkShrStatement(left, src1Op, src2Op);
add_instr(new IR::DpdkShrStatement(left, src1Op, src2Op));
} else if (right->is<IR::Equ>()) {
i = new IR::DpdkEquStatement(left, src1Op, src2Op);
add_instr(new IR::DpdkEquStatement(left, src1Op, src2Op));
} else if (right->is<IR::LOr>() || right->is<IR::LAnd>()) {
process_logical_operation(left, r);
} else if (right->is<IR::BOr>()) {
i = new IR::DpdkOrStatement(left, src1Op, src2Op);
add_instr(new IR::DpdkOrStatement(left, src1Op, src2Op));
} else if (right->is<IR::BAnd>()) {
i = new IR::DpdkAndStatement(left, src1Op, src2Op);
add_instr(new IR::DpdkAndStatement(left, src1Op, src2Op));
} else if (right->is<IR::BXor>()) {
i = new IR::DpdkXorStatement(left, src1Op, src2Op);
add_instr(new IR::DpdkXorStatement(left, src1Op, src2Op));
} else {
BUG("%1% not implemented.", right);
}
/* If there is a binary operation present which involves non 8-bit aligned
fields , add BAnd operation with mask value of original bit-width to
avoid any result overflow */
if (!isEightBitAligned(left) && !isSignExtended) {
if (right->is<IR::Add>() || right->is<IR::Sub>() ||
right->is<IR::Shl>() || right->is<IR::Shr>()) {
auto width = left->type->width_bits();
auto mask = (1 << width) - 1;
add_instr(new IR::DpdkAndStatement(left, left, new IR::Constant(mask)));
}
}
} else if (auto m = right->to<IR::MethodCallExpression>()) {
auto mi = P4::MethodInstance::resolve(m, refmap, typemap);
if (auto e = mi->to<P4::ExternMethod>()) {
Expand Down
6 changes: 6 additions & 0 deletions backends/dpdk/dpdkUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,10 @@ bool isMetadataStruct(const IR::Type_Struct *st) {
return false;
}

bool isEightBitAligned(const IR::Expression *e) {
if (e->type->width_bits() % 8 != 0)
return false;
return true;
}

} // namespace DPDK
1 change: 1 addition & 0 deletions backends/dpdk/dpdkUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ bool isNonConstantSimpleExpression(const IR::Expression *e);
bool isCommutativeBinaryOperation(const IR::Operation_Binary *bin);
bool isStandardMetadata(cstring name);
bool isMetadataStruct(const IR::Type_Struct *st);
bool isEightBitAligned(const IR::Expression *e);
} // namespace DPDK
#endif /* BACKENDS_DPDK_DPDKUTILS_H_ */
146 changes: 146 additions & 0 deletions testdata/p4_16_samples/psa-subtract-inst1.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/* -*- P4_16 -*- */
#include <core.p4>
#include <psa.p4>

/************ H E A D E R S ******************************/
#define MAX_LAYERS 2

struct EMPTY {};

header ethernet_t {
bit<48> dst_addr;
bit<48> src_addr;
bit<16> ether_type;
}

header udp_t {
bit<16> sport;
bit<16> dport;
bit<16> length;
bit<16> csum;
}

struct headers_t {
ethernet_t ethernet;
udp_t[MAX_LAYERS] udp;
}

struct user_meta_data_t {
bit<48> addr;
bit<3> depth1;
bit<5> depth2;
bit<5> depth3;
bit<5> depth4;
bit<5> depth5;
bit<5> depth6;
bit<5> depth7;
}

/*************************************************************************
**************** I N G R E S S P R O C E S S I N G *****************
*************************************************************************/
parser MyIngressParser(
packet_in pkt,
out headers_t hdr,
inout user_meta_data_t m,
in psa_ingress_parser_input_metadata_t c,
in EMPTY d,
in EMPTY e) {

state start {
m.depth1 = m.depth1 - 1;
pkt.extract(hdr.ethernet);
transition accept;
}
}

control MyIngressControl(
inout headers_t hdrs,
inout user_meta_data_t meta,
in psa_ingress_input_metadata_t c,
inout psa_ingress_output_metadata_t d) {
bit<5> var1 = 8;
bit<5> var2 = 2;
int<33> var3;
int<33> var4 = var3 - 3;
action nonDefAct() {
if (var4 == 3) {
meta.depth3 = meta.depth3 - 3;
meta.depth4 = var1 + var2;
meta.depth5 = var1 ^ 2;
meta.depth6 = var2 - 1;
meta.depth7 = var2 + 3;
}
}

table stub {
key = {}

actions = {
nonDefAct;
}
const default_action = nonDefAct;
size=1000000;
}

apply {
meta.depth4 = meta.depth2 - 4;
d.egress_port = (PortId_t) ((bit <32>) c.ingress_port ^ 1);
stub.apply();
}
}

control MyIngressDeparser(
packet_out pkt,
out EMPTY a,
out EMPTY b,
out EMPTY c,
inout headers_t hdr,
in user_meta_data_t e,
in psa_ingress_output_metadata_t f) {

apply {
pkt.emit(hdr.ethernet);
}
}

/*************************************************************************
**************** E G R E S S P R O C E S S I N G *******************
*************************************************************************/
parser MyEgressParser(
packet_in pkt,
out EMPTY a,
inout EMPTY b,
in psa_egress_parser_input_metadata_t c,
in EMPTY d,
in EMPTY e,
in EMPTY f) {
state start {
transition accept;
}
}

control MyEgressControl(
inout EMPTY a,
inout EMPTY b,
in psa_egress_input_metadata_t c,
inout psa_egress_output_metadata_t d) {
apply {}
}
control MyEgressDeparser(
packet_out pkt,
out EMPTY a,
out EMPTY b,
inout EMPTY c,
in EMPTY d,
in psa_egress_output_metadata_t e,
in psa_egress_deparser_input_metadata_t f) {
apply {}
}

/************ F I N A L P A C K A G E ******************************/
IngressPipeline(MyIngressParser(), MyIngressControl(), MyIngressDeparser()) ip;

EgressPipeline(MyEgressParser(), MyEgressControl(), MyEgressDeparser()) ep;

PSA_Switch(ip, PacketReplicationEngine(), ep, BufferingQueueingEngine()) main;
8 changes: 8 additions & 0 deletions testdata/p4_16_samples_outputs/pna-example-varIndex-1.p4.spec
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ action execute_1 args none {
jmpneq LABEL_FALSE_2 m.local_metadata_depth 0x0
mov m.MainControlT_tmp_1 m.local_metadata_depth
add m.MainControlT_tmp_1 0x3
and m.MainControlT_tmp_1 0x3
jmpneq LABEL_FALSE_3 m.MainControlT_tmp_1 0x0
jmp LABEL_END_3
LABEL_FALSE_3 : mov m.MainControlT_tmp_0 m.local_metadata_depth
add m.MainControlT_tmp_0 0x3
and m.MainControlT_tmp_0 0x3
jmpneq LABEL_FALSE_4 m.MainControlT_tmp_0 0x1
mov m.MainControlT_tmp_5 h.vlan_tag_0.pcp_cfi_vid
and m.MainControlT_tmp_5 0xf
Expand All @@ -79,6 +81,7 @@ action execute_1 args none {
jmp LABEL_END_3
LABEL_FALSE_4 : mov m.MainControlT_tmp m.local_metadata_depth
add m.MainControlT_tmp 0x3
and m.MainControlT_tmp 0x3
jmplt LABEL_END_3 m.MainControlT_tmp 0x1
mov m.MainControlT_tmp_11 h.vlan_tag_0.pcp_cfi_vid
and m.MainControlT_tmp_11 0xf
Expand All @@ -93,6 +96,7 @@ action execute_1 args none {
LABEL_FALSE_2 : jmpneq LABEL_END_3 m.local_metadata_depth 0x1
mov m.MainControlT_tmp_4 m.local_metadata_depth
add m.MainControlT_tmp_4 0x3
and m.MainControlT_tmp_4 0x3
jmpneq LABEL_FALSE_7 m.MainControlT_tmp_4 0x0
mov m.MainControlT_tmp_15 h.vlan_tag_1.pcp_cfi_vid
and m.MainControlT_tmp_15 0xf
Expand All @@ -109,10 +113,12 @@ action execute_1 args none {
jmp LABEL_END_3
LABEL_FALSE_7 : mov m.MainControlT_tmp_3 m.local_metadata_depth
add m.MainControlT_tmp_3 0x3
and m.MainControlT_tmp_3 0x3
jmpneq LABEL_FALSE_8 m.MainControlT_tmp_3 0x1
jmp LABEL_END_3
LABEL_FALSE_8 : mov m.MainControlT_tmp_2 m.local_metadata_depth
add m.MainControlT_tmp_2 0x3
and m.MainControlT_tmp_2 0x3
jmplt LABEL_END_3 m.MainControlT_tmp_2 0x1
mov m.MainControlT_tmp_21 h.vlan_tag_1.pcp_cfi_vid
and m.MainControlT_tmp_21 0xf
Expand Down Expand Up @@ -146,10 +152,12 @@ apply {
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG : extract h.vlan_tag_0
add m.local_metadata_depth 0x3
and m.local_metadata_depth 0x3
jmpeq MAINPARSERIMPL_PARSE_VLAN_TAG1 h.vlan_tag_0.ether_type 0x8100
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG1 : extract h.vlan_tag_1
add m.local_metadata_depth 0x3
and m.local_metadata_depth 0x3
jmpeq MAINPARSERIMPL_PARSE_VLAN_TAG2 h.vlan_tag_1.ether_type 0x8100
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG2 : mov m.pna_pre_input_metadata_parser_error 0x3
Expand Down
2 changes: 2 additions & 0 deletions testdata/p4_16_samples_outputs/pna-example-varIndex-2.p4.spec
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,12 @@ apply {
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG : extract h.vlan_tag_0
add m.local_metadata_depth 0x3
and m.local_metadata_depth 0x3
jmpeq MAINPARSERIMPL_PARSE_VLAN_TAG1 h.vlan_tag_0.ether_type 0x8100
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG1 : extract h.vlan_tag_1
add m.local_metadata_depth 0x3
and m.local_metadata_depth 0x3
jmpeq MAINPARSERIMPL_PARSE_VLAN_TAG2 h.vlan_tag_1.ether_type 0x8100
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG2 : mov m.pna_pre_input_metadata_parser_error 0x3
Expand Down
7 changes: 7 additions & 0 deletions testdata/p4_16_samples_outputs/pna-example-varIndex.p4.spec
Original file line number Diff line number Diff line change
Expand Up @@ -64,25 +64,30 @@ regarray direction size 0x100 initval 0
action execute_1 args none {
mov m.MainControlT_tmp_1 m.local_metadata_depth
add m.MainControlT_tmp_1 0x3
and m.MainControlT_tmp_1 0x3
jmpneq LABEL_FALSE_1 m.MainControlT_tmp_1 0x0
mov m.local_metadata_ethType h.vlan_tag_0.ether_type
jmp LABEL_END_2
LABEL_FALSE_1 : mov m.MainControlT_tmp_0 m.local_metadata_depth
add m.MainControlT_tmp_0 0x3
and m.MainControlT_tmp_0 0x3
jmpneq LABEL_FALSE_2 m.MainControlT_tmp_0 0x1
mov m.local_metadata_ethType h.vlan_tag_1.ether_type
jmp LABEL_END_2
LABEL_FALSE_2 : mov m.MainControlT_tmp m.local_metadata_depth
add m.MainControlT_tmp 0x3
and m.MainControlT_tmp 0x3
jmplt LABEL_END_2 m.MainControlT_tmp 0x1
mov m.local_metadata_ethType m.MainControlT_hsVar
LABEL_END_2 : mov m.MainControlT_tmp_3 m.local_metadata_depth
add m.MainControlT_tmp_3 0x3
and m.MainControlT_tmp_3 0x3
jmpneq LABEL_FALSE_4 m.MainControlT_tmp_3 0x0
mov h.vlan_tag_0.ether_type 0x2
jmp LABEL_END_5
LABEL_FALSE_4 : mov m.MainControlT_tmp_2 m.local_metadata_depth
add m.MainControlT_tmp_2 0x3
and m.MainControlT_tmp_2 0x3
jmpneq LABEL_END_5 m.MainControlT_tmp_2 0x1
mov h.vlan_tag_1.ether_type 0x2
LABEL_END_5 : jmpneq LABEL_FALSE_6 m.local_metadata_depth 0x0
Expand Down Expand Up @@ -164,10 +169,12 @@ apply {
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG : extract h.vlan_tag_0
add m.local_metadata_depth 0x3
and m.local_metadata_depth 0x3
jmpeq MAINPARSERIMPL_PARSE_VLAN_TAG1 h.vlan_tag_0.ether_type 0x8100
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG1 : extract h.vlan_tag_1
add m.local_metadata_depth 0x3
and m.local_metadata_depth 0x3
jmpeq MAINPARSERIMPL_PARSE_VLAN_TAG2 h.vlan_tag_1.ether_type 0x8100
jmp MAINPARSERIMPL_ACCEPT
MAINPARSERIMPL_PARSE_VLAN_TAG2 : mov m.pna_pre_input_metadata_parser_error 0x3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ apply {
mov m.Ingress_tmp_6 h.ipv4.version_ihl
mov m.Ingress_tmp_7 m.Ingress_tmp_6
add m.Ingress_tmp_7 0x5
and m.Ingress_tmp_7 0xf
mov m.Ingress_tmp_8 m.Ingress_tmp_7
mov m.Ingress_tmp_9 m.Ingress_tmp_8
and m.Ingress_tmp_9 0xf
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ apply {
mov m.Ingress_tmp_6 h.ipv4.version_ihl
mov m.Ingress_tmp_7 m.Ingress_tmp_6
add m.Ingress_tmp_7 0x5
and m.Ingress_tmp_7 0xf
mov m.Ingress_tmp_8 m.Ingress_tmp_7
mov m.Ingress_tmp_9 m.Ingress_tmp_8
and m.Ingress_tmp_9 0xf
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ apply {
mov m.Ingress_tmp_6 h.ipv4.version_ihl
mov m.Ingress_tmp_7 m.Ingress_tmp_6
add m.Ingress_tmp_7 0x5
and m.Ingress_tmp_7 0xf
mov m.Ingress_tmp_8 m.Ingress_tmp_7
mov m.Ingress_tmp_9 m.Ingress_tmp_8
and m.Ingress_tmp_9 0xf
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ apply {
mov m.Ingress_tmp_8 h.ipv4._version0__ihl1
mov m.Ingress_tmp_9 m.Ingress_tmp_8
add m.Ingress_tmp_9 0x5
and m.Ingress_tmp_9 0xf
mov m.Ingress_tmp_10 m.Ingress_tmp_9
mov m.Ingress_tmp_11 m.Ingress_tmp_10
and m.Ingress_tmp_11 0xf
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ apply {
mov m.Ingress_tmp_12 h.ipv4._version0__ihl1
mov m.Ingress_tmp_13 m.Ingress_tmp_12
add m.Ingress_tmp_13 0x5
and m.Ingress_tmp_13 0xf
mov m.Ingress_tmp_14 m.Ingress_tmp_13
mov m.Ingress_tmp_15 m.Ingress_tmp_14
and m.Ingress_tmp_15 0xf
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ apply {
mov m.Ingress_tmp_8 h.ipv4._version0__ihl1
mov m.Ingress_tmp_9 m.Ingress_tmp_8
add m.Ingress_tmp_9 0x5
and m.Ingress_tmp_9 0xf
mov m.Ingress_tmp_10 m.Ingress_tmp_9
mov m.Ingress_tmp_11 m.Ingress_tmp_10
and m.Ingress_tmp_11 0xf
Expand Down
Loading

0 comments on commit 40cb5a0

Please sign in to comment.