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

Remove ReferenceMap from majority of midend passes #4936

Merged
merged 22 commits into from
Oct 2, 2024
Merged

Conversation

asl
Copy link
Contributor

@asl asl commented Oct 2, 2024

I believe this covers "easy" cases, e.g. when refmap is not used, where it is used as name generator or trivial ResolutionContext.

Few issues were found, e.g. PassManager did not forward the parent context.

asl added 20 commits October 1, 2024 10:25
…t all

Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
@asl asl added core Topics concerning the core segments of the compiler (frontend, midend, parser) run-validation Use this tag to trigger a Validation CI run. run-sanitizer Use this tag to run a Clang+Sanitzers CI run. breaking-change This change may break assumptions of compiler back ends. labels Oct 2, 2024
@asl asl requested review from grg, vlstill, ChrisDodd and fruffy October 2, 2024 04:21
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
@@ -54,13 +54,11 @@ void DpdkBackend::convert(const IR::ToplevelBlock *tlb) {
new DpdkArchFirst(),
new ValidateOperandSize(),
new CollectLocalStructAndFlatten(refMap, typeMap),
new P4::EliminateTypedef(refMap, typeMap),
new P4::ClearTypeMap(typeMap),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we no longer need to clear the type map?

Edit: I see that ClearTypeMap has been pushed into EliminateTypedef.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly. It is better to be explicit :)

Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
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.

Looks great

Now to get rid of the typemap everywhere...

@asl asl added this pull request to the merge queue Oct 2, 2024
Merged via the queue into p4lang:main with commit 49e835c Oct 2, 2024
18 checks passed
@asl asl deleted the midend-refmap branch October 2, 2024 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This change may break assumptions of compiler back ends. core Topics concerning the core segments of the compiler (frontend, midend, parser) run-sanitizer Use this tag to run a Clang+Sanitzers CI run. run-validation Use this tag to trigger a Validation CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants