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

Midend def-use pass #4489

Merged
merged 5 commits into from
Mar 16, 2024
Merged

Midend def-use pass #4489

merged 5 commits into from
Mar 16, 2024

Conversation

grg
Copy link
Contributor

@grg grg commented Feb 29, 2024

Contributing a midend def-use analysis pass.

The pass is an inspector named ComputeDefUse. The pass populates a private attribute named defuse. This defuse information is queried via one of two functions: getDefs and getUses. Both functions take a const IR::Node* parameter and return the defs or uses for that node.

There is also a ostream operator that takes a ComputeDefUse object and prints the defuse information in human-readable form. For example:

defs:
  h.h1.f1<146088>(control:4:17): { <93152>(control:0:47) }
  inout ParsedHeaders h<93152>(control:0:47): { <93152>(control:0:47), <146097>(control:5:17), <146080>(control:2:13) }
  inout Metadata m<93156>(control:0:65): { <93156>(control:0:65) }
uses:
  inout ParsedHeaders h<93152>(control:0:47): { <146088>(control:4:17), <93152>(control:0:47) }
  h.h1.f1<146097>(control:5:17): { <93152>(control:0:47) }
  h.h1.f2<146080>(control:2:13): { <93152>(control:0:47) }
  inout Metadata m<93156>(control:0:65): { <93156>(control:0:65) }

The defs sections show for each element what the corresponding defs are (e.g., the use h.h1.f1 on line 4 of control has a corresponding def on line 0 of control). Similarly, the uses section shows for each element what the corresponding uses are.

This pass has not been extensively tested.

@ChrisDodd The second, third, and fourth commits are changes on top of the original def-use code: they fix a few issues that were uncovered while testing:

  • The second fixes an issue where split_slice is passed a range that doesn't correspond to any existing slices.
  • The third sets liveness from the input of a control block, and propagates liveness in do_write while drilling down into structlike types.
  • The fourth attempts to distinguish between control blocks that are "type declarations" vs actual block instantiations and doesn't attempt to set field liveness for type declarations.

The test issue1914-1.p4 provides an example of a control block type declaration:

control EmptyVerifyChecksum<H, M>(inout H hdr, inout M meta) {
    apply { }
}

The midend def-use pass sees this as:

  [96958] P4Control name=EmptyVerifyChecksum declid=17
    type: [250] Type_Control name=EmptyVerifyChecksum declid=16
      annotations: [5] Annotations
      typeParameters: [238] TypeParameters
        [235] Type_Var name=H declid=14
        [236] Type_Var name=M declid=15
      applyParams: [248] ParameterList
        [242] Parameter name=hdr declid=37 direction=inout
          annotations: [5] Annotations
          type: [241] Type_Name
            path: [240] Path name=H absolute=0
        [246] Parameter name=meta declid=38 direction=inout
          annotations: [5] Annotations
          type: [245] Type_Name
            path: [244] Path name=M absolute=0
    constructorParams: [258] ParameterList
    body: [96979] BlockStatement
      annotations: [5] Annotations

Without the protection in the fourth commit, the pass would call resolveUnique on H and M which both fail.

@grg grg force-pushed the grgibb/midend_def_use branch 6 times, most recently from 75a1c72 to 4fb2858 Compare March 1, 2024 03:24
@grg grg removed request for hanw, ChrisDodd and fruffy March 1, 2024 04:04
Copy link
Collaborator

@fruffy fruffy left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot for contributing this! Quick initial comments.

ir/CMakeLists.txt Outdated Show resolved Hide resolved
@grg grg changed the base branch from main to grgibb/bitrange_update March 2, 2024 03:35
@grg grg force-pushed the grgibb/midend_def_use branch 2 times, most recently from a2c1f38 to e4f6e68 Compare March 2, 2024 03:53
@grg grg changed the base branch from grgibb/bitrange_update to main March 2, 2024 03:55
@grg grg force-pushed the grgibb/midend_def_use branch 2 times, most recently from 05d38ac to 3ffef19 Compare March 4, 2024 21:10
@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Mar 5, 2024
@grg grg marked this pull request as ready for review March 8, 2024 01:44
@grg grg requested review from fruffy and ChrisDodd March 8, 2024 01:44
Copy link
Collaborator

@fruffy fruffy left a comment

Choose a reason for hiding this comment

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

I am surprised this code does not have any impact on existing ref files.

@grg
Copy link
Contributor Author

grg commented Mar 8, 2024

I am surprised this code does not have any impact on existing ref files.

Why do you say that? The information collected by this pass in the p4test backend is not currently being used by any subsequent passes.

@fruffy
Copy link
Collaborator

fruffy commented Mar 8, 2024

I am surprised this code does not have any impact on existing ref files.

Why do you say that? The information collected by this pass in the p4test backend is not currently being used by any subsequent passes.

Nevermind, for some reason I thought this would be used by a latter pass. Ignore my drive-by comment.

@grg
Copy link
Contributor Author

grg commented Mar 8, 2024

I am surprised this code does not have any impact on existing ref files.

Why do you say that? The information collected by this pass in the p4test backend is not currently being used by any subsequent passes.

Nevermind, for some reason I thought this would be used by a latter pass. Ignore my drive-by comment.

Ideally it will be used by a latter pass. We've just got to wait for someone to write that latter pass 🙂

@grg grg requested a review from asl March 9, 2024 19:17
Copy link
Contributor

@ChrisDodd ChrisDodd left a comment

Choose a reason for hiding this comment

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

Overall I think this is not quite fully-baked and needs more testing, but getting it in will make that easier to do.

midend/def_use.h Outdated Show resolved Hide resolved
@asl
Copy link
Contributor

asl commented Mar 14, 2024

@grg Just to understand the place of this pass as compared to frontend use-def pass. Is it possible to provide some explanation, etc. about the intended usage?

grg and others added 2 commits March 15, 2024 13:16
Modify split_slice to add slices when the range doesn't overlap with
exisitng slices.
grg added 3 commits March 15, 2024 15:50
Fixes:
 - Mark input parameters as live upon entring a control. (Not trying to
   propagate liveness information across parser/control boundaries.)
 - In do_write, when recursively calling do_write for structlike
   objects, copy struct liveness to all the fields before calling
   do_write on the target field.
@grg
Copy link
Contributor Author

grg commented Mar 15, 2024

@grg Just to understand the place of this pass as compared to frontend use-def pass. Is it possible to provide some explanation, etc. about the intended usage?

I believe the front-end def-use passes identify all the defs of fields (ComputeWriteSet in def_use.h/cpp), and identify which writes have uses (FindUnitialized in simplifyDefUse.cpp), but I don't think these record where the defs are used.

This new pass computes the defs and uses for each field, allowing for queries of the form "give me all the defs for node A" or "give me all the uses for node B".

Similar defuse information is used in the Tofino compiler for things like live range analysis, which is part of the PHV allocation (roughly register allocation), identifying uninitialized reads coming from P4 code, and additional unused field elimination. The Tofino code uses a backend def-use pass to do this, but Chris has implemented this midend version before switching roles to allow some of this functionality/additional functionality to be placed in the midend.

@ChrisDodd Feel free to correct anything I've got wrong or elaborate where you feel it might be useful.

@grg grg added this pull request to the merge queue Mar 15, 2024
Merged via the queue into main with commit ade47f4 Mar 16, 2024
17 checks passed
@grg grg deleted the grgibb/midend_def_use branch March 16, 2024 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants