Combo of duplication action name check and LocalizeAllActions fix #4975

jafingerhut · 2024-10-22T20:50:11Z

I have updated this initial comment with the status of the PR after 52 commits. The most recent changes were simply to update the C++ implementation so they worked after #4992 was merged in.

This PR is a combination of the changes in what was originally two separate PRs, but it seems to me easier to manage if they are combined and reviewed as one. The two earlier PRs were #4951 and #4970. Some of the discussion on comments in #4951 may still provide valuable context.

These changes are intended to address the problem where p4c currently allows a user to have identical @name annotations on multiple objects, e.g. on multiple actions. This can cause incorrect P4Info files to be generated, among other likely problems resulting from that root cause. It also addresses a minor bug in the LocalizeAllActions pass that produces wrong @name annotations for some top-level actions. See this slide deck for examples and more details:

https://docs.google.com/presentation/d/1JqQciG8hWdd2UlbXbDENw36yfGGRk2LugjzRrbv6Sf4/edit?usp=sharing

The code changes here are based upon this PR, which stalled a while ago: #3228

These changes are noticeably different from that PR, however, in that this one adds a new pass that occurs before the LocalizeAllActions pass, which is the pass where p4c by design can synthesize actions that are duplicates of user-written actions, all of which have identical @name annotations. Unless we change that behavior, any checks for duplicate @name annotations must be done before that pass. Such a change is described verbally in this comment on the PR above: #3228 (comment) but to my knowledge has not been implemented before. This PR is trying to implement that approach.

It passes all of the CI tests, after modifying a handful of P4 programs that had the error being checked for, so they no longer had the error. One of those was preserved to verify that p4c catches the error with the expected error messages.

The new pass is only enabled if one or more of the command line options are given to generate P4Info output files. The duplicate @name annotations checked for in the new pass only cause problems (that I am aware of) if P4Info output files are being generated. This is similar to some existing checks for duplicate names of P4 objects in the control plane serialization code for P4Info.

There have been multiple related issues filed on p4c over several years that this PR might address, at least partially if not completely.

These issues I believe are fully fixed by this PR:

p4c should prohibit different controllable entities having the same control-plane name #4651
- I have re-read this issue after commit 24 on this PR, and this PR should fully address the case where a P4 developer writes two actions with the same @name annotation. It does not change the fact that LocalizeAllActions can make copies of a single action in the IR, where each of those copies have equal @name annotations. I have invited kfcripps to review this PR. If this PR is merged, I believe issue 4651 should be closed, or at least updated to reflect that the first sub-issue there is fixed, but the example in the second comment is not (or create a separate issue for p4c should prohibit different controllable entities having the same control-plane name #4651's second sub-issue).
Incorrect P4Info generation when different actions have same name annotation #2755
- I have re-read issue 2755 after commit 24 on this PR, and this PR should fully address it. If this PR is merged, I believe issue 2755 should be closed, since the issue will be fixed.
Compiler silently allows duplicate name annotations #1949
- I have re-read this issue after commit 24 on this PR, and this PR should fully address that issue. I have re-tested the one test program that gave an erroneous P4Info output file attached to a comment on that issue, and with this PR's changes that program gives a clear error message about the duplicate @name annotations if you attempt to generate P4Info from it. There is a similar P4 program in this PR, so no need to add that particular test program to this PR. If this PR is merged, I believe issue 1949 should be closed, since the issue will be fixed.
Pass LocalizeAllActions neglects to prepend a "." to @name annotations that do not have one when it adds global actions into controls #4969
- The fixes for this issue are entirely in the LocalizeAllActions pass.

If this pull request is merged, then this earlier (apparently abandoned) PR should be closed:

Add check of uniqueness of control plane names #3228

The issue below should remain open even if this PR is merged, as the issue is really asking questions about what names should be used for controllable P4 objects in a P4Info file. This PR does not change any names in P4Info files -- it only gives errors and prevents P4Info files from being generated when the current names would conflict:

Is this the intended effect of @name annotations when generating P4Info files? #2716
- I have re-read this issue after commit 24 on this PR, and I believe I should not have linked to it from this PR at all. The questions raised in issue 2716 are definitely related to name annotations and how they affect the contents of P4Info files, but if there are any bugs there, this PR is not attempting to change the contents of correctly-generated P4Info files at all -- it is only attempting to give errors when P4Info files should not be generated.

I still need to re-read and think about whether the following issues are fixed by this PR, or whether there are other questions in them that are not fixed by this PR:

Control-plane name clash introduced by LocalizeAllActions and RemoveActionParameters #2749

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

…ce files that are the output of the midend pass of the compiler, because they can include intentional name duplication created in the LocalizeActions pass. + P4-16 source files created by translating a P4-14 source file to P4-16. Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu> Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Explicitly allow a table and action to have the same name, since there is a test case that was written explicitly to test that this is permitted. Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

and for some others, update the error messages we expect to see. Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

…rchical-name-check-pass

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

…om:jafingerhut/p4c into add-duplicate-hierarchical-name-check-pass

…rchical-name-check-pass

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

…rchical-name-check-pass

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

@name

so that it prepends a "." to @name annotations of global actions, if they do not have one already. Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

…ations Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

jafingerhut · 2024-11-08T19:16:15Z

testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4

@@ -0,0 +1,185 @@
+/*


@chrispsommers @smolkaj This is one of the two new P4 test programs I would recommend you focus your review on. The comments indicate which parts of the program I expect should give errors, and you can look at the corresponding testdata/p4_16_errors_output/actions-with-duplicate-control-plane-names1.p4-stderr file here to see that it does give errors for those: https://github.com/p4lang/p4c/pull/4975/files#diff-30d824dc2073bd520ea90c54fc52935bad924ddc326b1ab76e225e9c94bacae5

jafingerhut · 2024-11-08T19:18:15Z

testdata/p4_16_samples/actions-almost-duplicate-names1.p4

@@ -0,0 +1,173 @@
+/*


@chrispsommers @smolkaj This is the second of the two new P4 test programs I would recommend you focus your review on. The comments indicate which parts of the program I expect should not give errors, and they do not. You might also want to look through the contents of the expected generated P4Info file named testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb here: https://github.com/p4lang/p4c/pull/4975/files#diff-958b36defef5fa0260fb1c4424a06e6f725d05c1853b99070b691a48ee458c7b

…n-action-name-check-and-localizeallactions-fix

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

... the recent annotations infra changes Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

oleg-ran-amd

@jafingerhut Thanks for taking cate of this!
I have not completed the tests yet,
Please find a few comments for the C++ part below.

oleg-ran-amd · 2024-12-02T14:05:14Z

frontends/p4/duplicateActionControlPlaneNameCheck.h

+#include "ir/ir.h"
+#include "ir/visitor.h"


#include <vector>

Added in commit 57

oleg-ran-amd · 2024-12-02T14:08:11Z

frontends/p4/duplicateActionControlPlaneNameCheck.h

+        visitDagOnce = false;
+    }
+    const IR::Node *preorder(IR::P4Parser *parser) override {
+        stack.push_back(getName(parser));


What if we call prune() to prevent the visitor from going deeper into the parser which cannot have actions?

I think, pushing the parser onto the stack can be removed as we never use parser names to build hierarchical action names.

Agreed there should be no need to walk through sub-nodes within a parser.

Changed in commit 57

oleg-ran-amd · 2024-12-02T14:08:55Z

frontends/p4/duplicateActionControlPlaneNameCheck.h

+    const IR::Node *postorder(IR::P4Parser *parser) override {
+        stack.pop_back();
+        return parser;
+    }


If we apply the change proposed in the previous comment, this overload can be removed, I believe.

Changed in commit 57

oleg-ran-amd · 2024-12-02T14:17:19Z

frontends/p4/duplicateActionControlPlaneNameCheck.h

+    const IR::Node *preorder(IR::P4Table *table) override {
+        visit(table->annotations);
+        prune();
+        return table;
+    }


Can you please elaborate why we need to visit the table's annotations here?

It most likely is not needed, and I will try removing it. This looks like something left over from when I started by copying-and-pasting the HierarchicalNames pass, and never noticed it could/should be removed.

I left the method, but removed the visit call, but left the call to prune, since there should be no need to traverse the sub-nodes of a P4 table.

oleg-ran-amd · 2024-12-02T14:23:46Z

frontends/p4/duplicateActionControlPlaneNameCheck.h

+class DuplicateActionControlPlaneNameCheck : public Transform {
+    std::vector<cstring> stack;
+    /// Used for detection of conflicting control plane names among actions.
+    string_map<const IR::Node *> actions;


string_map preserves the insertion order.
This is not needed for this PR as it never iterates through the elements.
Keeping the order also consumes extra resources (std::list + its memory allocations).
What if we replace this with absl::flat_hash_map?

Changes in commit 57

oleg-ran-amd · 2024-12-02T14:28:51Z

frontends/p4/duplicateActionControlPlaneNameCheck.cpp

+}
+
+const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *action) {
+    bool topLevel = findContext<IR::P4Control>() == nullptr;


bool topLevel = !stack.empty();

has some chances to be a tiny bit more efficient, I believe.
(this requires the previously suggested change for not pushing parsers onto the stack).

Changed to bool topLevel = stack.empty(); in commit 57

asl · 2024-12-02T15:01:59Z

frontends/p4/duplicateActionControlPlaneNameCheck.cpp

+        cstring name = absl::StrCat(".", action->name);
+        checkForDuplicateName(name, action);
+    } else {
+        const auto *e0 = nameAnno->expr.at(0);


Suggested change

const auto *e0 = nameAnno->expr.at(0);

const auto *e0 = nameAnno->getExpr(0);

Changed in commit 55, IIRC

asl · 2024-12-02T15:03:21Z

frontends/p4/duplicateActionControlPlaneNameCheck.cpp

+                                                                 const IR::Node *node) {
+    bool foundDuplicate = false;
+    auto *otherNode = node;
+    auto [it, inserted] = actions.insert(std::pair(name, node));


Suggested change

auto [it, inserted] = actions.insert(std::pair(name, node));

auto [it, inserted] = actions.emplace(name, node);

Changed in commit 57

asl · 2024-12-02T15:03:47Z

frontends/p4/duplicateActionControlPlaneNameCheck.cpp

+    auto [it, inserted] = actions.insert(std::pair(name, node));
+    if (!inserted) {
+        foundDuplicate = true;
+        otherNode = (*it).second;


Suggested change

otherNode = (*it).second;

otherNode = it->second;

Changed in commit 57

asl · 2024-12-02T15:04:11Z

frontends/p4/duplicateActionControlPlaneNameCheck.cpp

+        foundDuplicate = true;
+        otherNode = (*it).second;
+    }
+    if (foundDuplicate) {


Could probably fold into !inserted condition above

Changed in commit 57

frontends/p4/duplicateActionControlPlaneNameCheck.cpp

asl · 2024-12-02T15:08:10Z

frontends/p4/localizeActions.cpp

+        } else {
+            cstring name = absl::StrCat(".", action->name);
+            action->addAnnotationIfNew(IR::Annotation::nameAnnotation, new IR::StringLiteral(name),
+                                       false);


false is default, could be omitted.

Removed in commit 57

asl · 2024-12-02T15:08:35Z

frontends/p4/localizeActions.cpp

+            // If the value of the existing name annotation does not
+            // begin with ".", prepend "." so that the name remains
+            // global if control plane APIs are generated later.
+            const auto *e0 = nameAnno->expr.at(0);


Suggested change

const auto *e0 = nameAnno->expr.at(0);

const auto *e0 = nameAnno->getExpr(0);

Changed in commit 55, IIRC

asl · 2024-12-02T15:09:07Z

frontends/p4/localizeActions.cpp

-        cstring name = absl::StrCat(".", action->name);
-        action->addAnnotationIfNew(IR::Annotation::nameAnnotation, new IR::StringLiteral(name),
-                                   false);
+        auto nameAnno = action->getAnnotation(IR::Annotation::nameAnnotation);


Could move declaration into if:

if (const auto *nameAnno = ...)

Changed in commit 57

…-localizeallactions-fix

…tion Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

jafingerhut · 2024-12-02T19:49:48Z

@jafingerhut Thanks for taking cate of this! I have not completed the tests yet, Please find a few comments for the C++ part below.

No worries. This is an issue I noticed way back in 2019, and it has occasionally bugged me over the years whenever I notice it again. Glad if we can get it fixed.

Thanks for the comments. If it is not obvious, C++ is not something I would call myself an expert in. I believe I have addressed the current round of comments completely in commits today.

While I believe these changes address the questions in the initial comment of this issue: #4651

it does not attempt to make any changes regarding Kyle's follow-up comment here: #4651 (comment)

As I have tried to state elsewhere, that is caused by the LocalizeAllActions pass, and it is by design.

I would recommend either:

(a) realize that this is what LocalizeAllActions does, and write later passes, and your target-specific back end code, to take it into account that there can be multiple actions with the same name, but with the changes in this PR, I believe we can guarantee that all actions with the same hierarchical name were created by LocalizeAllActions, not by the P4 developer.

OR

(b) figure out a way in your version of p4c to avoid using the LocallizeAllActions pass. That is not as simple as just commenting out that pass, I don't think.

kfcripps

I have reviewed only the C++ changes + the testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 test and related outputs. I will let others review the rest of the changes to the tests outputs. Aside from one suggestion, the parts that I have reviewed LGTM.

frontends/p4/duplicateActionControlPlaneNameCheck.cpp

frontends/p4/duplicateActionControlPlaneNameCheck.h

kfcripps · 2024-12-03T01:17:27Z

frontends/common/options.h

+    /// Indicates whether control plane API generation is enabled.
+    /// @returns default to false unless a command line option was
+    /// given explicitly enabling control plane API generation.
+    virtual bool controlPlaneAPIGenEnabled() const {
+        if (p4RuntimeFile.isNullOrEmpty() && p4RuntimeFiles.isNullOrEmpty() &&
+            p4RuntimeEntriesFile.isNullOrEmpty() && p4RuntimeEntriesFiles.isNullOrEmpty()) {
+            return false;
+        }
+        return true;
+    }


@asl @oleg-ran-amd I think that DuplicateActionControlPlaneNameCheck should run for our backend even when P4Info files are not generated. So we should probably override this function to unconditionally return true.

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

…n-action-name-check-and-localizeallactions-fix

jafingerhut and others added 30 commits October 8, 2024 21:54

Add pass to give error if duplicate hierarchical names are present

e429c8a

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Add new source files to CMakeLists.txt

39538bf

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Skip new checks for P4-14 source files in run-bmv2-test.py

a2792ce

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Fix lint error

a553fe1

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Fix lint problems.

26f0501

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu> Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Do not give errors for variable declarations with identical names

da456fe

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Limit search for duplicate names to actions, tables, or "other objects"

b683571

Explicitly allow a table and action to have the same name, since there is a test case that was written explicitly to test that this is permitted. Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Update test programs that had errors in their name annotations

ad40fe9

and for some others, update the error messages we expect to see. Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Update expected output files for modified test programs

91166c8

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Move new test program with error into p4_16_errors directory

9acb7e0

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Add expected output files for new test programs

b133b28

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

More fine-tuning to enable tests to pass

76bf24f

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Fix lint error

f7ec2fe

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Fix lint error

1cc8224

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Merge branch 'master' into add-duplicate-hierarchical-name-check-pass

4b03eb5

Address some review comments

93daba5

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Rewording documentation of new pass for clarity.

66a0a0a

Signed-off-by: Andy Fingerhut <andy.fingerhut@gmail.com>

Merge remote-tracking branch 'upstream/main' into add-duplicate-hiera…

083db34

…rchical-name-check-pass

Enable the new pass only if P4Runtime control plane API gen is enabled

99beb30

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Merge branch 'add-duplicate-hierarchical-name-check-pass' of github.c…

2b7c58a

…om:jafingerhut/p4c into add-duplicate-hierarchical-name-check-pass

Merge remote-tracking branch 'upstream/main' into add-duplicate-hiera…

54007df

…rchical-name-check-pass

Undo changes to Python test scripts

d89f214

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Undo changes to gtestp4c source

7424db3

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Merge remote-tracking branch 'upstream/main' into add-duplicate-hiera…

55ad18f

…rchical-name-check-pass

Use abseil StrCat and StrAppend instead of loop

799448a

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Fix formatting with clang-format

377f41a

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Fix p4lang#4969 by modifying pass LocalizeActions

682564b

so that it prepends a "." to @name annotations of global actions, if they do not have one already. Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Fix clang-format error

bb88b7d

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Also catch duplicates involving top-level actions without @name annot…

d265e1c

…ations Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

jafingerhut commented Nov 8, 2024

View reviewed changes

jafingerhut added 5 commits November 9, 2024 19:25

Merge remote-tracking branch 'upstream/main' into combo-of-duplicatio…

e563b90

…n-action-name-check-and-localizeallactions-fix

Updates due to recent changes in annotation APIs

62ee2c1

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

More modifications required to make the code work with ...

c3feb19

... the recent annotations infra changes Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Simplify string manipulation code using latest abseil changes

31f0b10

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Fix clang-format warning

bd31e6b

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

kfcripps requested review from oleg-ran-amd and removed request for oleg-ran-amd November 14, 2024 18:48

oleg-ran-amd reviewed Dec 2, 2024

View reviewed changes

asl reviewed Dec 2, 2024

View reviewed changes

frontends/p4/duplicateActionControlPlaneNameCheck.cpp Outdated Show resolved Hide resolved

asl reviewed Dec 2, 2024

View reviewed changes

jafingerhut added 6 commits December 2, 2024 15:12

Merge branch 'master' into combo-of-duplication-action-name-check-and…

a853313

…-localizeallactions-fix

Updates required to work with recent changes to annotation implementa…

c111ab1

…tion Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Address review comments

b542540

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Removed an old line of commented-out code.

beb87b0

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Reword some comments for clarity.

87feba6

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Fix lint warning

e586849

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

kfcripps reviewed Dec 3, 2024

View reviewed changes

frontends/p4/duplicateActionControlPlaneNameCheck.cpp Outdated Show resolved Hide resolved

frontends/p4/duplicateActionControlPlaneNameCheck.h Show resolved Hide resolved

kfcripps reviewed Dec 3, 2024

View reviewed changes

jafingerhut added 2 commits December 3, 2024 03:06

Address review comment

381ec48

Signed-off-by: Andy Fingerhut <andy_fingerhut@alum.wustl.edu>

Merge remote-tracking branch 'upstream/main' into combo-of-duplicatio…

2f861cf

…n-action-name-check-and-localizeallactions-fix

kfcripps requested a review from oleg-ran-amd December 11, 2024 00:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combo of duplication action name check and LocalizeAllActions fix #4975

Combo of duplication action name check and LocalizeAllActions fix #4975

jafingerhut commented Oct 22, 2024 •

edited

Loading

jafingerhut Nov 8, 2024

jafingerhut Nov 8, 2024

oleg-ran-amd left a comment

oleg-ran-amd Dec 2, 2024

jafingerhut Dec 2, 2024

oleg-ran-amd Dec 2, 2024

jafingerhut Dec 2, 2024

jafingerhut Dec 2, 2024

oleg-ran-amd Dec 2, 2024

jafingerhut Dec 2, 2024

oleg-ran-amd Dec 2, 2024

jafingerhut Dec 2, 2024

jafingerhut Dec 2, 2024

oleg-ran-amd Dec 2, 2024

jafingerhut Dec 2, 2024

oleg-ran-amd Dec 2, 2024

jafingerhut Dec 2, 2024

asl Dec 2, 2024

jafingerhut Dec 2, 2024

asl Dec 2, 2024

jafingerhut Dec 2, 2024

asl Dec 2, 2024

jafingerhut Dec 2, 2024

asl Dec 2, 2024

jafingerhut Dec 2, 2024

asl Dec 2, 2024

jafingerhut Dec 2, 2024

asl Dec 2, 2024

jafingerhut Dec 2, 2024

asl Dec 2, 2024

jafingerhut Dec 2, 2024

jafingerhut commented Dec 2, 2024

kfcripps left a comment

kfcripps Dec 3, 2024

	const auto *e0 = nameAnno->expr.at(0);
	const auto *e0 = nameAnno->getExpr(0);

	auto [it, inserted] = actions.insert(std::pair(name, node));
	auto [it, inserted] = actions.emplace(name, node);

		#include "ir/ir.h"
		#include "ir/visitor.h"

Combo of duplication action name check and LocalizeAllActions fix #4975

Are you sure you want to change the base?

Combo of duplication action name check and LocalizeAllActions fix #4975

Conversation

jafingerhut commented Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleg-ran-amd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jafingerhut commented Dec 2, 2024

kfcripps left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jafingerhut commented Oct 22, 2024 •

edited

Loading