Add an option to copy structures to skip expanding header unions.

Signed-off-by: fruffy <fruffy@nyu.edu>
p4lang · fruffy · Aug 5, 2024 · Aug 7, 2024 · Aug 5, 2024 · Oct 25, 2024
commit 17df75bc9c83aeb34d9a1883c1765c5eea899df1
diff --git a/backends/bmv2/pna_nic/midend.cpp b/backends/bmv2/pna_nic/midend.cpp
@@ -130,7 +130,7 @@ PnaNicMidEnd::PnaNicMidEnd(CompilerOptions &options, std::ostream *outStream)
             new P4::StrengthReduction(&typeMap),
             new P4::EliminateTuples(&typeMap),
             new P4::SimplifyComparisons(&typeMap),
-            new P4::CopyStructures(&typeMap),
+            new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
             new P4::NestedStructs(&typeMap),
             new P4::SimplifySelectList(&typeMap),
             new P4::RemoveSelectBooleans(&typeMap),

diff --git a/backends/bmv2/psa_switch/midend.cpp b/backends/bmv2/psa_switch/midend.cpp
@@ -131,7 +131,7 @@ PsaSwitchMidEnd::PsaSwitchMidEnd(CompilerOptions &options, std::ostream *outStre
             new P4::StrengthReduction(&typeMap),
             new P4::EliminateTuples(&typeMap),
             new P4::SimplifyComparisons(&typeMap),
-            new P4::CopyStructures(&typeMap),
+            new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
             new P4::NestedStructs(&typeMap),
             new P4::SimplifySelectList(&typeMap),
             new P4::RemoveSelectBooleans(&typeMap),

diff --git a/backends/bmv2/simple_switch/midend.cpp b/backends/bmv2/simple_switch/midend.cpp
@@ -98,7 +98,7 @@ SimpleSwitchMidEnd::SimpleSwitchMidEnd(CompilerOptions &options, std::ostream *o
              new P4::StrengthReduction(&typeMap),
              new P4::EliminateTuples(&typeMap),
              new P4::SimplifyComparisons(&typeMap),
-             new P4::CopyStructures(&typeMap),
+             new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
              new P4::NestedStructs(&typeMap),
              new P4::SimplifySelectList(&typeMap),
              new P4::RemoveSelectBooleans(&typeMap),

diff --git a/backends/dpdk/midend.cpp b/backends/dpdk/midend.cpp
@@ -194,7 +194,11 @@ DpdkMidEnd::DpdkMidEnd(CompilerOptions &options, std::ostream *outStream) {
             new P4::StrengthReduction(&typeMap),
             new P4::EliminateTuples(&typeMap),
             new P4::SimplifyComparisons(&typeMap),
-            new P4::CopyStructures(&typeMap, false /* errorOnMethodCall */),
+            new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig{
+                                                 /*errorOnMethodCall*/ false,
+                                                 /*copyHeaders*/ false,
+                                                 /*expandUnions*/ true,
+                                             }),
             new P4::NestedStructs(&typeMap),
             new P4::SimplifySelectList(&typeMap),
             new P4::RemoveSelectBooleans(&typeMap),

diff --git a/backends/p4test/midend.cpp b/backends/p4test/midend.cpp
@@ -102,7 +102,11 @@ MidEnd::MidEnd(CompilerOptions &options, std::ostream *outStream) {
          new P4::StrengthReduction(&typeMap),
          new P4::EliminateTuples(&typeMap),
          new P4::SimplifyComparisons(&typeMap),
-         new P4::CopyStructures(&typeMap, false),
+         new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig{
+                                              /*errorOnMethodCall*/ false,
+                                              /*copyHeaders*/ false,
+                                              /*expandUnions*/ true,
+                                          }),
          new P4::NestedStructs(&typeMap),
          new P4::StrengthReduction(&typeMap),
          new P4::SimplifySelectList(&typeMap),

diff --git a/backends/p4tools/common/compiler/midend.cpp b/backends/p4tools/common/compiler/midend.cpp
@@ -121,7 +121,13 @@ void MidEnd::addDefaultPasses() {
         new P4::SimplifyComparisons(&typeMap),
         // Expand header and struct assignments into sequences of field assignments.
         new PassRepeated({
-            new P4::CopyStructures(&typeMap, false, true, nullptr),
+            new P4::CopyStructures(&typeMap,
+                                   {
+                                       /*errorOnMethodCall*/ false,
+                                       /*copyHeaders*/ true,
+                                       /*expandUnions*/ false,
+                                   },
+                                   nullptr),
         }),
         new P4::RemoveParserControlFlow(&typeMap),
         // Flatten nested list expressions.

diff --git a/backends/ubpf/midend.cpp b/backends/ubpf/midend.cpp
@@ -93,7 +93,7 @@ const IR::ToplevelBlock *MidEnd::run(EbpfOptions &options, const IR::P4Program *
                 new P4::StrengthReduction(&typeMap),
             }),
             new P4::SimplifyComparisons(&typeMap),
-            new P4::CopyStructures(&typeMap),
+            new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
             new P4::LocalCopyPropagation(&typeMap),
             new P4::SimplifySelectList(&typeMap),
             new P4::MoveDeclarations(),  // more may have been introduced

diff --git a/midend/copyStructures.cpp b/midend/copyStructures.cpp
@@ -73,13 +73,14 @@ const IR::Node *DoCopyStructures::postorder(IR::AssignmentStatement *statement)
        FIXME: this is not correct for header unions and should be fixed.
        The fix bellow, commented-out, causes problems elsewhere.
        https://github.com/p4lang/p4c/issues/3842
-    if (ltype->is<IR::Type_HeaderUnion>())
-        return statement;
     */
+    if (!_config.expandUnions && ltype->is<IR::Type_HeaderUnion>()) {
+        return statement;
+    }
 
     // Do not copy structures for method calls.
     if (statement->right->is<IR::MethodCallExpression>()) {
-        if (errorOnMethodCall) {
+        if (_config.errorOnMethodCall) {
             ::P4::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
                         "%1%: functions or methods returning structures "
                         "are not supported on this target",
@@ -98,7 +99,7 @@ const IR::Node *DoCopyStructures::postorder(IR::AssignmentStatement *statement)
             retval->push_back(
                 new IR::AssignmentStatement(statement->srcInfo, left, right->expression));
         }
-    } else if (copyHeaders && ltype->is<IR::Type_Header>()) {
+    } else if (_config.copyHeaders && ltype->is<IR::Type_Header>()) {
         const auto *header = ltype->checkedTo<IR::Type_Header>();
         // Build a "src.isValid()" call.
         const auto *isSrcValidCall = new IR::MethodCallExpression(

diff --git a/midend/copyStructures.h b/midend/copyStructures.h
@@ -22,6 +22,21 @@ limitations under the License.
 
 namespace P4 {
 
+struct CopyStructuresConfig {
+    /// Specific targets may allow functions or methods to return structs.
+    /// Such methods will not be converted in this pass. Setting the
+    /// errorOnMethodCall flag will produce an error message if such a
+    ///  method is encountered.
+    bool errorOnMethodCall = true;
+
+    /// Do not only copy normal structures but also perform copy assignments for headers.
+    bool copyHeaders = false;
+    /// Also expand header union assignments.
+    /// TODO: This is only necessary because the copy structure pass does not correctly expand
+    /// header unions for some back ends.
+    bool expandUnions = true;
+};
+
 /**
  * Convert assignments between structures to assignments between fields
  *
@@ -53,19 +68,15 @@ namespace P4 {
  *
  */
 class DoCopyStructures : public Transform {
+    /// The type map.
     TypeMap *typeMap;
-    /// Specific targets may allow functions or methods to return structs.
-    /// Such methods will not be converted in this pass. Setting the
-    /// errorOnMethodCall flag will produce an error message if such a
-    ///  method is encountered.
-    bool errorOnMethodCall;
 
-    /// Do not only copy normal structures but also perform copy assignments for headers.
-    bool copyHeaders;
+    /// Configuration options.
+    CopyStructuresConfig _config;
 
  public:
-    explicit DoCopyStructures(TypeMap *typeMap, bool errorOnMethodCall, bool copyHeaders = false)
-        : typeMap(typeMap), errorOnMethodCall(errorOnMethodCall), copyHeaders(copyHeaders) {
+    explicit DoCopyStructures(TypeMap *typeMap, CopyStructuresConfig config)
+        : typeMap(typeMap), _config(config) {
         CHECK_NULL(typeMap);
         setName("DoCopyStructures");
     }
@@ -116,15 +127,15 @@ class RemoveAliases : public Transform {
 
 class CopyStructures : public PassRepeated {
  public:
-    explicit CopyStructures(TypeMap *typeMap, bool errorOnMethodCall = true,
-                            bool copyHeaders = false, TypeChecking *typeChecking = nullptr) {
+    explicit CopyStructures(TypeMap *typeMap, CopyStructuresConfig config,
+                            TypeChecking *typeChecking = nullptr) {
         CHECK_NULL(typeMap);
         setName("CopyStructures");
         if (typeChecking == nullptr) typeChecking = new TypeChecking(nullptr, typeMap);
         passes.emplace_back(typeChecking);
         passes.emplace_back(new RemoveAliases(typeMap));
         passes.emplace_back(typeChecking);
-        passes.emplace_back(new DoCopyStructures(typeMap, errorOnMethodCall, copyHeaders));
+        passes.emplace_back(new DoCopyStructures(typeMap, config));
     }
 };
 

diff --git a/test/gtest/midend_pass.cpp b/test/gtest/midend_pass.cpp
@@ -83,7 +83,7 @@ MidEnd::MidEnd(CompilerOptions &options, std::ostream *outStream) {
          new P4::StrengthReduction(&typeMap),
          new P4::EliminateTuples(&typeMap),
          new P4::SimplifyComparisons(&typeMap),
-         new P4::CopyStructures(&typeMap),
+         new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
          new P4::NestedStructs(&typeMap),
          new P4::StrengthReduction(&typeMap),
          new P4::SimplifySelectList(&typeMap),