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.

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

Fix incorrect copy propagation into table match keys #3399

Merged
merged 2 commits into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion midend/local_copyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -585,8 +585,10 @@ void DoLocalCopyPropagation::apply_function(DoLocalCopyPropagation::FuncInfo *ac

void DoLocalCopyPropagation::apply_table(DoLocalCopyPropagation::TableInfo *tbl) {
++tbl->apply_count;
std::unordered_set<cstring> remaps_seen;
for (auto key : tbl->keyreads) {
forOverlapAvail(key, [key, tbl, this](cstring vname, VarInfo *var) {
forOverlapAvail(key, [&remaps_seen, key, tbl, this](cstring vname, VarInfo *var) {
remaps_seen.insert(vname);
if (var->val && lvalue_out(var->val)->is<IR::PathExpression>()) {
if (tbl->apply_count > 1 &&
(!tbl->key_remap.count(vname) || !tbl->key_remap.at(vname)->equiv(*var->val))) {
Expand All @@ -606,6 +608,12 @@ void DoLocalCopyPropagation::apply_table(DoLocalCopyPropagation::TableInfo *tbl)
LOG4(" table using " << key << " with " <<
(var->val ? "value to complex for key" : "no propagated value"));
var->live = true; } }); }
for (auto it = tbl->key_remap.begin(); it != tbl->key_remap.end(); ) {
if (remaps_seen.count(it->first) == 0) {
LOG3(" no value used for some applies for key " << it->first);
it = tbl->key_remap.erase(it);
} else {
++it; } }
for (auto action : tbl->actions)
apply_function(&actions[action]);
}
Expand Down
65 changes: 65 additions & 0 deletions testdata/p4_16_samples/copyprop1.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#include <core.p4>
#include <v1model.p4>

header payload_t {
bit<8> x;
bit<8> y;
}
struct header_t {
payload_t payload;
}
struct metadata {}

parser MyParser(packet_in packet,
out header_t hdr,
inout metadata meta,
inout standard_metadata_t standard_metadata) {
state start {
packet.extract(hdr.payload);
transition accept;
}
}

control MyIngress(inout header_t hdr,
inout metadata meta,
inout standard_metadata_t standard_metadata) {

action a1() {
hdr.payload.x = 0xaa;
}

table t1 {
key = { hdr.payload.x : exact; }
actions = { a1; }
size = 1024;
}

apply {
if (hdr.payload.y == 0) {
hdr.payload.x = hdr.payload.y;
t1.apply();
} else {
t1.apply();
}
standard_metadata.egress_spec = 2;
}
}

control MyVerifyChecksum(inout header_t hdr, inout metadata meta) { apply { } }
control MyEgress(inout header_t hdr, inout metadata meta,
inout standard_metadata_t standard_metadata) { apply { } }
control MyDeparser(packet_out packet, in header_t hdr) {
apply {
packet.emit(hdr);
}
}
control MyComputeChecksum(inout header_t hdr, inout metadata meta) { apply { } }

V1Switch(
MyParser(),
MyVerifyChecksum(),
MyIngress(),
MyEgress(),
MyComputeChecksum(),
MyDeparser()
) main;
72 changes: 72 additions & 0 deletions testdata/p4_16_samples_outputs/copyprop1-first.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
#include <core.p4>
#define V1MODEL_VERSION 20180101
#include <v1model.p4>

header payload_t {
bit<8> x;
bit<8> y;
}

struct header_t {
payload_t payload;
}

struct metadata {
}

parser MyParser(packet_in packet, out header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
state start {
packet.extract<payload_t>(hdr.payload);
transition accept;
}
}

control MyIngress(inout header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
action a1() {
hdr.payload.x = 8w0xaa;
}
table t1 {
key = {
hdr.payload.x: exact @name("hdr.payload.x") ;
}
actions = {
a1();
@defaultonly NoAction();
}
size = 1024;
default_action = NoAction();
}
apply {
if (hdr.payload.y == 8w0) {
hdr.payload.x = hdr.payload.y;
t1.apply();
} else {
t1.apply();
}
standard_metadata.egress_spec = 9w2;
}
}

control MyVerifyChecksum(inout header_t hdr, inout metadata meta) {
apply {
}
}

control MyEgress(inout header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
apply {
}
}

control MyDeparser(packet_out packet, in header_t hdr) {
apply {
packet.emit<header_t>(hdr);
}
}

control MyComputeChecksum(inout header_t hdr, inout metadata meta) {
apply {
}
}

V1Switch<header_t, metadata>(MyParser(), MyVerifyChecksum(), MyIngress(), MyEgress(), MyComputeChecksum(), MyDeparser()) main;

74 changes: 74 additions & 0 deletions testdata/p4_16_samples_outputs/copyprop1-frontend.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#include <core.p4>
#define V1MODEL_VERSION 20180101
#include <v1model.p4>

header payload_t {
bit<8> x;
bit<8> y;
}

struct header_t {
payload_t payload;
}

struct metadata {
}

parser MyParser(packet_in packet, out header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
state start {
packet.extract<payload_t>(hdr.payload);
transition accept;
}
}

control MyIngress(inout header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@noWarn("unused") @name(".NoAction") action NoAction_1() {
}
@name("MyIngress.a1") action a1() {
hdr.payload.x = 8w0xaa;
}
@name("MyIngress.t1") table t1_0 {
key = {
hdr.payload.x: exact @name("hdr.payload.x") ;
}
actions = {
a1();
@defaultonly NoAction_1();
}
size = 1024;
default_action = NoAction_1();
}
apply {
if (hdr.payload.y == 8w0) {
hdr.payload.x = hdr.payload.y;
t1_0.apply();
} else {
t1_0.apply();
}
standard_metadata.egress_spec = 9w2;
}
}

control MyVerifyChecksum(inout header_t hdr, inout metadata meta) {
apply {
}
}

control MyEgress(inout header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
apply {
}
}

control MyDeparser(packet_out packet, in header_t hdr) {
apply {
packet.emit<header_t>(hdr);
}
}

control MyComputeChecksum(inout header_t hdr, inout metadata meta) {
apply {
}
}

V1Switch<header_t, metadata>(MyParser(), MyVerifyChecksum(), MyIngress(), MyEgress(), MyComputeChecksum(), MyDeparser()) main;

92 changes: 92 additions & 0 deletions testdata/p4_16_samples_outputs/copyprop1-midend.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
#include <core.p4>
#define V1MODEL_VERSION 20180101
#include <v1model.p4>

header payload_t {
bit<8> x;
bit<8> y;
}

struct header_t {
payload_t payload;
}

struct metadata {
}

parser MyParser(packet_in packet, out header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
state start {
packet.extract<payload_t>(hdr.payload);
transition accept;
}
}

control MyIngress(inout header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@noWarn("unused") @name(".NoAction") action NoAction_1() {
}
@name("MyIngress.a1") action a1() {
hdr.payload.x = 8w0xaa;
}
@name("MyIngress.t1") table t1_0 {
key = {
hdr.payload.x: exact @name("hdr.payload.x") ;
}
actions = {
a1();
@defaultonly NoAction_1();
}
size = 1024;
default_action = NoAction_1();
}
@hidden action copyprop1l39() {
hdr.payload.x = hdr.payload.y;
}
@hidden action copyprop1l44() {
standard_metadata.egress_spec = 9w2;
}
@hidden table tbl_copyprop1l39 {
actions = {
copyprop1l39();
}
const default_action = copyprop1l39();
}
@hidden table tbl_copyprop1l44 {
actions = {
copyprop1l44();
}
const default_action = copyprop1l44();
}
apply {
if (hdr.payload.y == 8w0) {
tbl_copyprop1l39.apply();
t1_0.apply();
} else {
t1_0.apply();
}
tbl_copyprop1l44.apply();
}
}

control MyVerifyChecksum(inout header_t hdr, inout metadata meta) {
apply {
}
}

control MyEgress(inout header_t hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
apply {
}
}

control MyDeparser(packet_out packet, in header_t hdr) {
apply {
packet.emit<payload_t>(hdr.payload);
}
}

control MyComputeChecksum(inout header_t hdr, inout metadata meta) {
apply {
}
}

V1Switch<header_t, metadata>(MyParser(), MyVerifyChecksum(), MyIngress(), MyEgress(), MyComputeChecksum(), MyDeparser()) main;

Loading