Support for 128 bit bitwise operation #4952

Sosutha · 2024-10-09T04:42:40Z

Added movh statement definition
Added support for 128 bitwise operation
Updated testcase and output

psivanup

Please check the comments

psivanup · 2024-10-09T09:39:51Z

backends/dpdk/dpdkArch.h

+                }
+            }
+        }
+        return false;


Is it valid input to have operand1 of 128 bits and operand2 less than 128 bits at this stage?
If true, isValidOperandSize in line number 1106 may issue error for operand being greater than 64 bits.
IMO, isValidOperandSize function can be parameterized to check limit of 128 bits in exceptional cases.

Only operations where both operands are of size 128 bits are being supported for now.

Typechecking will ensure that the operands to any binary operation like these are the exact same type, so there's really no need to check both. The only binops that can have different types for their operands are shifts, array index, and concat. If you want to be paranoid, you can BUG_CHECK(src1Op->type == src2Op->type, ...

Updated the code.

backends/dpdk/dpdkHelpers.cpp

psivanup · 2024-10-09T09:43:01Z

backends/dpdk/dpdkHelpers.cpp

+    fields->push_back(new IR::StructField("tmp", IR::Type_Bits::get(64)));
+    const IR::Type_Header *headerStruct = new IR::Type_Header(IR::ID("tmp128_t"), *fields);
+    auto name = new cstring("tmp128_t");
+    structure->header_types.emplace(*name, headerStruct);


Suggested change

structure->header_types.emplace(*name, headerStruct);

structure->header_types.emplace(cstring("tmp128_t"), headerStruct);

Updated the code

KrisNey-MSFT · 2024-10-09T16:10:38Z

Suggest to try to compile DASH code as a test of completeness to see if the code compiles/builds? If DASH has other than bitwise operations, it may fail. Do not support additions to IPv6 addresses (we would likely not do this), but all else s/be supported. Can we add @jafingerhut as a reviewer please?

jafingerhut · 2024-10-10T15:36:01Z

Updated on 2024-Oct-10 to correct the p4c-dpdk command line options. I still get errors saying 'Unupported bitwidth 128' in them with the updated command line.

I tried checking out this modified branch of the code, and built p4c-dpdk, then tried the following commands:

git clone https://github.com/sonic-net/DASH
cd DASH/dash-pipeline/bmv2
mkdir -p output
p4c-dpdk -DTARGET_DPDK_PNA -DPNA_CONNTRACK --pp output/dash_pipeline.pp.p4 \
        -o output/dash_pipeline.spec --arch pna \
        --bf-rt-schema output/dash_pipeline.p4.bfrt.json \
        --p4runtime-files output/dash_pipeline.p4.p4info.txt \
        dash_pipeline.p4

I see many warnings, which are unrelated to these changes, but I also see 7 errors "Unsupported bitwidth 128" as copied below:

stages/../dash_routing_types.p4(130): [--Werror=target-error] error: Unsupported bitwidth 128 in underlay_sip
                            underlay_sip = underlay_sip == 0 ? hdr.u0_ipv4.src_addr : (IPv4Addres...
                                           ^^^^^^^^^^^^
stages/../dash_routing_types.p4(130): [--Werror=target-error] error: Unsupported bitwidth 128 in 128w0
...                underlay_sip = underlay_sip == 0 ? hdr.u0_ipv4.src_addr : (IPv4Address)underla...
                                                  ^
stages/../dash_routing_types.p4(131): [--Werror=target-error] error: Unsupported bitwidth 128 in underlay_dip
                            underlay_dip = underlay_dip == 0 ? hdr.u0_ipv4.dst_addr : (IPv4Addres...
                                           ^^^^^^^^^^^^
stages/../dash_routing_types.p4(131): [--Werror=target-error] error: Unsupported bitwidth 128 in 128w0
...                underlay_dip = underlay_dip == 0 ? hdr.u0_ipv4.dst_addr : (IPv4Address)underla...
                                                  ^
stages/../dash_routing_types.p4(203): [--Werror=target-error] error: Unsupported bitwidth 128 in overlay_sip_mask
...( (IPv6Address)hdr.u0_ipv4.src_addr & ~overlay_sip_mask) | overlay_sip) & ~meta.eni_data.pl_si...
                                          ^^^^^^^^^^^^^^^^
[--Werror=target-error] error: Unsupported bitwidth 128 in meta._eni_data_pl_sip_mask20
dash_pipeline.p4(229): [--Werror=target-error] error: Unsupported bitwidth 128 in meta._overlay_data_dip_mask112
        routing_action_apply.apply(hdr, meta);
                                        ^^^^
dash_pipeline.p4(229): [--Werror=target-error] error: Unsupported bitwidth 128 in meta._overlay_data_sip_mask111
        routing_action_apply.apply(hdr, meta);
                                        ^^^^

Do you see these as well, Sosutha?

It would be good to understand why these errors occur, and if possible, either modify this PR so that those errors do not occur, or figure out a modified version of the DASH program that behaves as it is written today, but does not give those errors.

fruffy

This PR should not be merged until the compilator for https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples/dash/dash-pipeline-pna-dpdk.p4 passes

Signed-off-by: Sosutha Sethuramapandian <sosutha.sethuramapandian@intel.com> * Added movh statement definition * Added support for 128 bitwise operation * Updated testcase and output

Sosutha · 2024-10-11T11:51:32Z

p4_16_samples/dash/dash-pipeline-pna-dpdk.p4

Andy, I am getting same errors as you. This PR addresses only 128 bitwise operations. I can add implementation for comparison operations to this same PR. So errors related to that will be solved with that implementation. For error in apply statement, I will have to check and revert back

fruffy · 2024-10-12T19:14:57Z

p4_16_samples/dash/dash-pipeline-pna-dpdk.p4

Andy, I am getting same errors as you. This PR addresses only 128 bitwise operations. I can add implementation for comparison operations to this same PR. So errors related to that will be solved with that implementation. For error in apply statement, I will have to check and revert back

We can also add the failing compilation to XFails for now. The important part is that this program is added as a test that is checked.

Sosutha mentioned this pull request Oct 9, 2024

P4 DPDK on horizon; BMV2 sonic-net/DASH#408

Open

Sosutha force-pushed the dpdk_128_bit_support branch from 9e9f473 to a0e34e4 Compare October 9, 2024 04:44

Sosutha mentioned this pull request Oct 9, 2024

p4c-dpdk back end does not support 128-bit arithmetic #4174

Open

psivanup reviewed Oct 9, 2024

View reviewed changes

Sosutha force-pushed the dpdk_128_bit_support branch 3 times, most recently from e20762a to 30b7c86 Compare October 9, 2024 10:45

fruffy added the dpdk Topics related to the DPDK back end label Oct 9, 2024

fruffy requested a review from jafingerhut October 9, 2024 16:30

Sosutha force-pushed the dpdk_128_bit_support branch 5 times, most recently from 015e0a9 to 80518e8 Compare October 10, 2024 12:50

fruffy requested changes Oct 10, 2024

View reviewed changes

Sosutha force-pushed the dpdk_128_bit_support branch from 80518e8 to 0b866d7 Compare October 11, 2024 09:58

Support for 128 bit bitwise operation

ae74cb8

Signed-off-by: Sosutha Sethuramapandian <sosutha.sethuramapandian@intel.com> * Added movh statement definition * Added support for 128 bitwise operation * Updated testcase and output

Sosutha force-pushed the dpdk_128_bit_support branch from 0b866d7 to ae74cb8 Compare October 11, 2024 10:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for 128 bit bitwise operation #4952

Support for 128 bit bitwise operation #4952

Sosutha commented Oct 9, 2024

psivanup left a comment

psivanup Oct 9, 2024

Sosutha Oct 9, 2024

ChrisDodd Oct 9, 2024 •

edited

Loading

Sosutha Oct 10, 2024

psivanup Oct 9, 2024

Sosutha Oct 9, 2024

KrisNey-MSFT commented Oct 9, 2024 •

edited

Loading

jafingerhut commented Oct 10, 2024 •

edited

Loading

fruffy left a comment

Sosutha commented Oct 11, 2024

fruffy commented Oct 12, 2024

	structure->header_types.emplace(*name, headerStruct);
	structure->header_types.emplace(cstring("tmp128_t"), headerStruct);

Support for 128 bit bitwise operation #4952

Are you sure you want to change the base?

Support for 128 bit bitwise operation #4952

Conversation

Sosutha commented Oct 9, 2024

psivanup left a comment

Choose a reason for hiding this comment

psivanup Oct 9, 2024

Choose a reason for hiding this comment

Sosutha Oct 9, 2024

Choose a reason for hiding this comment

ChrisDodd Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Sosutha Oct 10, 2024

Choose a reason for hiding this comment

psivanup Oct 9, 2024

Choose a reason for hiding this comment

Sosutha Oct 9, 2024

Choose a reason for hiding this comment

KrisNey-MSFT commented Oct 9, 2024 • edited Loading

jafingerhut commented Oct 10, 2024 • edited Loading

fruffy left a comment

Choose a reason for hiding this comment

Sosutha commented Oct 11, 2024

fruffy commented Oct 12, 2024

ChrisDodd Oct 9, 2024 •

edited

Loading

KrisNey-MSFT commented Oct 9, 2024 •

edited

Loading

jafingerhut commented Oct 10, 2024 •

edited

Loading