-
Notifications
You must be signed in to change notification settings - Fork 446
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
Clarification question on uninitialized local headers #2383
Comments
My understanding is that your assumption described early in the comment is correct: The first code snippet will make the header The second code snippet will guarantee that |
Frontend pass setHeaders shifted after InlineFunctions pass, so that return values of functions could be processed as right operands of assignment statements, where left operand is header type or a header field. The idea was to remove the redundant setValid() calls in some outputs. The solution was tested on code given in issue 2383, and tests that had a duplicated setValid() call. With this change, there is no need for checking for header types when handling structures or lists in copyStructures pass in midend, because setValid() call is handled in frontend. There are no changes at the end of midend in test p4lang#2383 after implementing these changes, and tests with redundant setValid() calls have different outputs. For an example, in test issue2261.p4, there is no redundant setValid() call after midend anymore: @hidden action issue2261l22() { tmp_struct_0_eth_hdr.setValid(); tmp_struct_0_eth_hdr.dst_addr = 48w0; tmp_struct_0_eth_hdr.src_addr = 48w0; tmp_struct_0_eth_hdr.eth_type = 16w0; h.eth_hdr.eth_type = 16w0; }
Frontend pass setHeaders shifted after InlineFunctions pass, so that return values of functions could be processed as right operands of assignment statements, where left operand is header type or a header field. The idea was to remove the redundant setValid() calls in some outputs. The solution was tested on code given in issue 2383, and tests that had a duplicated setValid() call. With this change, there is no need for checking for header types when handling structures or lists in copyStructures pass in midend, because setValid() call is handled in frontend. There are no changes at the end of midend in test p4lang#2383 after implementing these changes, and tests with redundant setValid() calls have different outputs. For an example, in test issue2261.p4, there is no redundant setValid() call after midend anymore: @hidden action issue2261l22() { tmp_struct_0_eth_hdr.setValid(); tmp_struct_0_eth_hdr.dst_addr = 48w0; tmp_struct_0_eth_hdr.src_addr = 48w0; tmp_struct_0_eth_hdr.eth_type = 16w0; h.eth_hdr.eth_type = 16w0; }
After the recent discussions and spec merges I have a (hopefully) last question on header initialization. Are these constructs equivalent?
My initial guess was no, since the in the first example the header ist set to valid, whereas in the second I assume the header stays invalid. Otherwise any assignment to an invalid header would set it to valid again.
Based on my assumptions I have observed potentially incorrect behavior in the "CopyStructures" pass. The example program this time is actually something a sane person might write:
With
BMV2::SimpleSwitchMidEnd_19_CopyStructures
is converted to
Which lacks the
setValid
call. Maybe there is already a plan to fix this with thesetHeaders
pass?.header_function_cast.p4.txt
header_function_cast.stf.txt
The text was updated successfully, but these errors were encountered: