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

p4test: x: declaration not found #2435

Closed
fruffy opened this issue Jun 12, 2020 · 6 comments
Closed

p4test: x: declaration not found #2435

fruffy opened this issue Jun 12, 2020 · 6 comments
Assignees
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed. predication-pass Issues related to p4c's Predication pass

Comments

@fruffy
Copy link
Collaborator

fruffy commented Jun 12, 2020

Running p4test there seem to be a problem with the way variables are propagated in the mid end. This only happens in combination with exit statements. There are two examples. One happens after inlining a global action, the other happens after a constant key is simplified. In both cases MidEnd_25_LocalCopyPropagation removes variables that it should not.

key_after_exit.p4.txt

MidEnd_20_FlattenInterfaceStructs
MidEnd_21_ReplaceSelectRange
MidEnd_22_Predication
MidEnd_23_MoveDeclarations
MidEnd_24_ConstantFolding
MidEnd_25_LocalCopyPropagation
[--Werror=not-found] error: key_0: declaration not found
Done.*/

global_action_after_exit.p4.txt

MidEnd_20_FlattenInterfaceStructs
MidEnd_21_ReplaceSelectRange
MidEnd_22_Predication
MidEnd_23_MoveDeclarations
MidEnd_24_ConstantFolding
MidEnd_25_LocalCopyPropagation
bugs/crash/global_action_after_exit.p4(348): [--Werror=not-found] error: val: declaration not found
action do_action(inout ethernet_t val) {
@hesingh
Copy link
Contributor

hesingh commented Jun 12, 2020

@mbudiu-vmw and @ChrisDodd In our prior discussion, we seem to be converging on removing exit statements inside action if the action includes any hasOut parameter.

This issue uses an exit inside the control apply block and global action. I thought we could leave this exit for the bmv2 backend to deal with. If we do, the midend screws up the IR.

action do_action(inout ethernet_t val) {
    val.eth_type = 0xDEAD;
}
control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {

    apply {
        exit;
        do_action(h.eth_hdr);
    }
}

@mihaibudiu mihaibudiu added the bug This behavior is unintended and should be fixed. label Jun 16, 2020
@mihaibudiu mihaibudiu assigned mihaibudiu and ChrisDodd and unassigned mihaibudiu Jun 16, 2020
@mihaibudiu
Copy link
Contributor

@ChrisDodd : here the LocalCopyProp removes a variable used in a table key. So the dead code elimination is not conservative enough.

@hesingh hesingh mentioned this issue Jun 17, 2020
@ChrisDodd
Copy link
Contributor

ChrisDodd commented Jun 18, 2020

localCopyProp removes the key because the table is never applied, so it doesn't actually use the key. Unfortunately, as it doesn't remove the dead table, it leaves in the reference to the key in the table. For global_after_exited, the same applies, except it is the action that is dead, so the reference to the control local val in it is not noticed by local copyprop.

Easiest fix is probably to have a pass that removes unused tables and actions before local copyprop.

@mihaibudiu
Copy link
Contributor

Actually I believe that our pass to remove unused references should do this, but we explicitly prevent it from doing that, because this would change the P4Runtime API generated. I think dead tables should be removed.

@hesingh
Copy link
Contributor

hesingh commented Jun 18, 2020

This is why I developed the rmExits pass to remove dead tables and call rmExits just before unused references in frontend.cpp in my recent PR . The PR has fixed this issue.

@ChrisDodd
Copy link
Contributor

So I added a PR #2446 that avoids the problem in LocalCopyProp

  • by default, it will treat unreferenced actions/tables in a control as sources for potential access to local vars (so will not eliminate them)
  • with a flag, will remove the unreferenced local tables and actions (along with the other dead locals that were only referenced by them).

@fruffy fruffy closed this as completed Jun 23, 2020
@fruffy fruffy added predication-pass Issues related to p4c's Predication pass fixed This topic is considered to be fixed. labels Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed. predication-pass Issues related to p4c's Predication pass
Projects
None yet
Development

No branches or pull requests

4 participants