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

Dead code eliminator: use default if other actions won't be executed #56

Merged
merged 7 commits into from
Feb 24, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
12 changes: 12 additions & 0 deletions passes/elim_dead_code.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,18 @@ const IR::Node *ElimDeadCode::preorder(IR::MethodCallStatement *stmt) {
}
}

// Action annotated with `@defaultonly` is ignored before. If no action can be reachable
// based on previous analysis, we replace the apply call to the default action call.
// Only when the default action has an empty body, we remove the apply.
auto *defaultAction = table.getDefaultAction()->to<IR::MethodCallExpression>();
if (defaultAction != nullptr) {
auto decl = getActionDecl(refMap, *defaultAction);
if (decl.has_value() && !decl.value()->body->components.empty()) {
printInfo("Replacing table apply with default action %1%", defaultAction);
Copy link
Owner

Choose a reason for hiding this comment

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

Well, we can still add some form of eliminated node here. Otherwise we do not track that we have modified the table here. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

In the sense of eliminatedNodes.push_back(stmt); or something similar. Our current ref files do not quite capture this ambiguity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. I think a better way is to create another vector to store these because they are not simply eliminated. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

You could make it a vector of pairs, with the original node and the replaced node.

There is a question on the performance impact when we keep adding nodes. Recall that this code may run on every control-plane update message. But I do not think it will have much impact because we only invoke the elimination when the semantics have changed. Still, it's something we should keep in mind.

Copy link
Collaborator Author

@RabbitWhite1 RabbitWhite1 Feb 23, 2024

Choose a reason for hiding this comment

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

You could make it a vector of pairs, with the original node and the replaced node.

That sounds good. I will do it soon.

And yes, I agree on the performance thing.

return new IR::MethodCallStatement(stmt->getSourceInfo(), defaultAction);
}
}

// There is no action to execute other than an empty action, remove the table.
printInfo("Removing %1%", stmt);
eliminatedNodes.push_back(stmt);
Expand Down
2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/arith-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 35: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/arith1-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 35: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/arith2-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 35: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/arith3-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 35: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/arith4-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 35: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/arith5-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 35: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/checksum1-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 132: guh.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/concat-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 35: apply { t.apply(); }

Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ Eliminated node at line 576: if (use_dst_vnet_vni == 1) {
Eliminated node at line 665: } else if (packet_action == 1) {
Eliminated node at line 913: if (vip.apply().hit) {
Eliminated node at line 913: if (vip.apply().hit) {
Eliminated node at line 919: eni_ether_address_map.apply();
Eliminated node at line 924: vxlan_decap_pa_validate: {
Eliminated node at line 951: eni.apply();
Eliminated node at line (unknown):
Eliminated node at line 457: if (meta.stage1_dash_acl_group_id != 0) {
Eliminated node at line 467: if (meta.stage2_dash_acl_group_id != 0) {
Expand Down
2 changes: 0 additions & 2 deletions targets/bmv2/test/testdata/config/pins_middleblock.ref
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
Eliminated node at line 741: acl_pre_ingress_table.apply();
Eliminated node at line 412: vrf_table.apply();
Eliminated node at line 415: ipv4_table.apply();
Eliminated node at line 417: ipv6_table.apply();
Eliminated node at line 419: if (wcmp_group_id_valid) {
Eliminated node at line 422: if (nexthop_id_valid) {
Eliminated node at line 689: acl_ingress_table.apply();
Expand Down
3 changes: 0 additions & 3 deletions targets/bmv2/test/testdata/dash-pipeline-v1model-bmv2.ref
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,10 @@ Eliminated node at line 664: meta.dropped = true;
Eliminated node at line 665: } else if (packet_action == 1) {
Eliminated node at line 913: if (vip.apply().hit) {
Eliminated node at line 913: if (vip.apply().hit) {
Eliminated node at line 916: direction_lookup.apply();
Eliminated node at line 917: appliance.apply();
Eliminated node at line 918: meta.eni_addr = (meta.direction == dash_direction_t.OUTBOUND ? hdr.inner_ethernet.src_addr : hdr.inner_ethernet.dst_addr);
Eliminated node at line 919: eni_ether_address_map.apply();
Eliminated node at line 921: vxlan_decap(hdr);
Eliminated node at line 924: vxlan_decap_pa_validate: {
Eliminated node at line 951: eni.apply();
Eliminated node at line 955: acl_group.apply();
Eliminated node at line 957: outbound.apply(hdr, meta);
Eliminated node at line (unknown):
Expand Down
2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/drop-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 38: forward.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue-3312-graph-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 73: t2.apply();

1 change: 0 additions & 1 deletion targets/bmv2/test/testdata/issue1049-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 106: guh.apply();
Eliminated node at line 107: debug_table.apply();
2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue1062-1-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 69: t_exact.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue1538.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 87: mac_da.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue1544-1-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 82: mac_da.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue1544-2-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 80: mac_da.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue1544-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 80: mac_da.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue1713-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 46: apply { t.apply(); }

1 change: 0 additions & 1 deletion targets/bmv2/test/testdata/issue1739-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 117: ipv4_sa_filter.apply();
Eliminated node at line 118: ipv4_da_lpm.apply();
2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue1765-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 133: guh.apply();

1 change: 0 additions & 1 deletion targets/bmv2/test/testdata/issue298-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 164: drop_tbl.apply();
Eliminated node at line 187: round_tbl.apply();
2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue364-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 57: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/issue414-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 81: guh.apply();

4 changes: 1 addition & 3 deletions targets/bmv2/test/testdata/issue461-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
Eliminated node at line 127: ipv4_da_lpm.apply();
Eliminated node at line 128: mac_da.apply();
Eliminated node at line 150: send_frame.apply();

4 changes: 1 addition & 3 deletions targets/bmv2/test/testdata/issue561-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
Eliminated node at line 306: ipv4_da_lpm.apply();
Eliminated node at line 307: mac_da.apply();
Eliminated node at line 330: send_frame.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/parser-locals2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 97: guh.apply();

2 changes: 0 additions & 2 deletions targets/bmv2/test/testdata/pins_fabric.ref
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
Eliminated node at line 803: acl_pre_ingress_table.apply();
Eliminated node at line 554: l3_admit_table.apply();
Eliminated node at line 418: vrf_table.apply();
Eliminated node at line 421: ipv4_table.apply();
Eliminated node at line 423: ipv6_table.apply();
Eliminated node at line 425: if (wcmp_group_id_valid) {
Eliminated node at line 428: if (nexthop_id_valid) {
Eliminated node at line 750: acl_ingress_table.apply();
Expand Down
2 changes: 0 additions & 2 deletions targets/bmv2/test/testdata/pins_middleblock.ref
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
Eliminated node at line 741: acl_pre_ingress_table.apply();
Eliminated node at line 548: l3_admit_table.apply();
Eliminated node at line 412: vrf_table.apply();
Eliminated node at line 415: ipv4_table.apply();
Eliminated node at line 417: ipv6_table.apply();
Eliminated node at line 419: if (wcmp_group_id_valid) {
Eliminated node at line 422: if (nexthop_id_valid) {
Eliminated node at line 689: acl_ingress_table.apply();
Expand Down
2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/slice-def-use1.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 57: tbl_act.apply();

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/stack_complex-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 63: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/strength3.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 57: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/strength6.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 51: apply { t.apply(); }

2 changes: 1 addition & 1 deletion targets/bmv2/test/testdata/unused-counter-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 59: apply { t.apply(); }

4 changes: 1 addition & 3 deletions targets/bmv2/test/testdata/use-priority-as-name.ref
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
Eliminated node at line 133: ipv4_da_lpm.apply();
Eliminated node at line 134: mac_da.apply();
Eliminated node at line 162: send_frame.apply();

2 changes: 0 additions & 2 deletions targets/bmv2/test/testdata/v1model-special-ops-bmv2.ref
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
Eliminated node at line 290: if ((standard_metadata.instance_type == BMV2_V1MODEL_INSTANCE_TYPE_RESUBMIT)) {
Eliminated node at line 293: } else if ((standard_metadata.instance_type == BMV2_V1MODEL_INSTANCE_TYPE_RECIRC)) {
Eliminated node at line 297: ipv4_da_lpm.apply();
Eliminated node at line 299: if (meta.fwd.l2ptr != 0) {
Eliminated node at line 364: if ((standard_metadata.instance_type == BMV2_V1MODEL_INSTANCE_TYPE_INGRESS_CLONE)) {
Eliminated node at line 370: } else if ((standard_metadata.instance_type == BMV2_V1MODEL_INSTANCE_TYPE_EGRESS_CLONE)) {
Eliminated node at line 377: if ((standard_metadata.instance_type == BMV2_V1MODEL_INSTANCE_TYPE_REPLICATION)) {
Eliminated node at line 382: send_frame.apply();
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/bri_with_pdfixed_thrift.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 119: forward.apply();
Eliminated node at line 121: ipRouteMulticast.apply();
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_action_selector.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 127: forward.apply();
Eliminated node at line 128: set_dest.apply();
2 changes: 0 additions & 2 deletions targets/tofino/test/testdata/tna_custom_hash.ref
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
Eliminated node at line 141: output_port.apply();
Eliminated node at line 145: tbl_hash1.apply();
Eliminated node at line 146: tbl_hash2.apply();
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_digest.ref
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
Eliminated node at line 86: } else if (ig_dprsr_md.digest_type == 2) {
Eliminated node at line 154: smac.apply();
Eliminated node at line 165: dmac.apply();
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_dkm.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 153: forward.apply();
Eliminated node at line 155: ipRoute.apply();
2 changes: 1 addition & 1 deletion targets/tofino/test/testdata/tna_dyn_hashing.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 135: bloom_filter_1_0.apply();

1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_exact_match.ref
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
Eliminated node at line 168: forward.apply();
Eliminated node at line 170: ipRoute.apply();
Eliminated node at line 171: forward_timeout.apply();
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_lpm_match.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 128: forward.apply();
Eliminated node at line 129: alpm_forward.apply();
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_multicast.ref
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
Eliminated node at line 202: ing_port.apply();
Eliminated node at line 203: ing_src_ifid.apply();
Eliminated node at line 204: ing_dmac.apply();
Eliminated node at line 205: if (ig_md.l3 == 1) {
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_operations.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 130: forward.apply();
Eliminated node at line 131: forward_dst.apply();
3 changes: 1 addition & 2 deletions targets/tofino/test/testdata/tna_pktgen.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 119: t.apply();
Eliminated node at line 121: p.apply();

2 changes: 1 addition & 1 deletion targets/tofino/test/testdata/tna_port_metadata.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 126: port_md_exm_match.apply();

1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_ports.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 128: forward.apply();
Eliminated node at line 130: ipRoute.apply();
2 changes: 1 addition & 1 deletion targets/tofino/test/testdata/tna_random.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 61: bypass_egress.apply();

2 changes: 1 addition & 1 deletion targets/tofino/test/testdata/tna_range_match.ref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Eliminated node at line 102: forward.apply();

1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_simple_switch.ref
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ Eliminated node at line 217: validate_ipv6.apply();
Eliminated node at line 604: rmac_hit : {
Eliminated node at line 502: NoAction : { ecmp.apply(); }
Eliminated node at line 506: if (routed) {
Eliminated node at line 305: dmac.apply();
Eliminated node at line 549: lag.apply();
Eliminated node at line 797: ipv6_rewrite.apply();
Eliminated node at line 798: srh_rewrite.apply();
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_snapshot.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 133: forward.apply();
Eliminated node at line 134: ipRoute.apply();
1 change: 0 additions & 1 deletion targets/tofino/test/testdata/tna_symmetric_hash.ref
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Eliminated node at line 106: output_port.apply();
Eliminated node at line 61: bypass_egress.apply();
Loading