allow disabling of subtraction transform (#4633)

Add mechanism to allow the the tranform that changes
`a - const` -> `a + (-const)` to be disabled

frontends/p4/frontend.cpp

@@ -207,7 +207,7 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
         new TableKeyNames(&refMap, &typeMap),
         new PassRepeated({
             new ConstantFolding(&refMap, &typeMap, constantFoldingPolicy),
-            new StrengthReduction(&refMap, &typeMap),
+            new StrengthReduction(&refMap, &typeMap, policy->enableSubConstToAddTransform()),
             new Reassociation(),
             new UselessCasts(&refMap, &typeMap),
         }),

frontends/p4/frontend.h

@@ -48,6 +48,10 @@ class FrontEndPolicy : public RemoveUnusedPolicy {
     // TODO: This should probably not be allowed to be skipped at all.
     virtual bool skipSideEffectOrdering() const { return false; }
 
+    /// Indicates whether to enable the `a - constant` to `a + (-constant)` in StrengthReduction.
+    /// @returns Defaults to true.
+    virtual bool enableSubConstToAddTransform() const { return true; }
+
     /// Indicates whether the frontend should run some optimizations (inlining, action localization,
     /// etc.).
     /// @returns default to enabled optimizations unless -O0 was given in the options (i.e. enabled

frontends/p4/strengthReduction.cpp

@@ -182,7 +182,7 @@ const IR::Node *DoStrengthReduction::postorder(IR::Sub *expr) {
     if (isZero(expr->right)) return expr->left;
     if (isZero(expr->left)) return new IR::Neg(expr->srcInfo, expr->type, expr->right);
     // Replace `a - constant` with `a + (-constant)`
-    if (expr->right->is<IR::Constant>()) {
+    if (enableSubConstToAddTransform && expr->right->is<IR::Constant>()) {
         auto cst = expr->right->to<IR::Constant>();
         auto neg = new IR::Constant(cst->srcInfo, cst->type, -cst->value, cst->base, true);
         auto result = new IR::Add(expr->srcInfo, expr->type, expr->left, neg);

frontends/p4/strengthReduction.h

@@ -43,6 +43,11 @@ namespace P4 {
  *
  */
 class DoStrengthReduction final : public Transform {
+ protected:
+    /// Enable the subtract constant to add negative constant transform.
+    /// Replaces `a - constant` with `a + (-constant)`.
+    bool enableSubConstToAddTransform = true;
+
     /// @returns `true` if @p expr is the constant `1`.
     bool isOne(const IR::Expression *expr) const;
     /// @returns `true` if @p expr is the constant `0`.
@@ -70,6 +75,11 @@ class DoStrengthReduction final : public Transform {
         setName("StrengthReduction");
     }
 
+    explicit DoStrengthReduction(bool enableSubConstToAddTransform)
+        : enableSubConstToAddTransform(enableSubConstToAddTransform) {
+        DoStrengthReduction();
+    }
+
     using Transform::postorder;
 
     const IR::Node *postorder(IR::Cmpl *expr) override;
@@ -104,14 +114,19 @@ class DoStrengthReduction final : public Transform {
 
 class StrengthReduction : public PassManager {
  public:
-    StrengthReduction(ReferenceMap *refMap, TypeMap *typeMap,
-                      TypeChecking *typeChecking = nullptr) {
+    explicit StrengthReduction(ReferenceMap *refMap, TypeMap *typeMap,
+                               TypeChecking *typeChecking = nullptr,
+                               bool enableSubConstToAddTransform = true) {
         if (typeMap != nullptr) {
             if (!typeChecking) typeChecking = new TypeChecking(refMap, typeMap, true);
             passes.push_back(typeChecking);
         }
-        passes.push_back(new DoStrengthReduction());
+        passes.push_back(new DoStrengthReduction(enableSubConstToAddTransform));
     }
+
+    explicit StrengthReduction(ReferenceMap *refMap, TypeMap *typeMap,
+                               bool enableSubConstToAddTransform)
+        : StrengthReduction(refMap, typeMap, nullptr, enableSubConstToAddTransform) {}
 };
 
 }  // namespace P4

test/CMakeLists.txt

@@ -49,6 +49,7 @@ set (GTEST_UNITTEST_SOURCES
   gtest/path_test.cpp
   gtest/p4runtime.cpp
   gtest/source_file_test.cpp
+  gtest/strength_reduction.cpp
   gtest/transforms.cpp
   gtest/stringify.cpp
   gtest/rtti_test.cpp

test/gtest/strength_reduction.cpp

@@ -0,0 +1,116 @@
+#include <absl/strings/substitute.h>
+#include <gtest/gtest.h>
+
+#include <optional>
+
+#include "frontends/p4/frontend.h"
+#include "frontends/p4/strengthReduction.h"
+#include "frontends/p4/toP4/toP4.h"
+#include "helpers.h"
+#include "ir/dump.h"
+#include "ir/ir.h"
+#include "lib/sourceCodeBuilder.h"
+
+using namespace P4;
+
+namespace Test {
+
+namespace {
+
+std::optional<FrontendTestCase> createStrengthReductionTestCase(
+    const std::string &ingressSource, P4::FrontEndPolicy *policy = nullptr) {
+    std::string source = P4_SOURCE(P4Headers::V1MODEL, R"(
+header H
+{
+   bit<32> f1;
+   bit<32> f2;
+   bit<32> f3;
+}
+
+struct Headers { H h; }
+struct Metadata { }
+
+parser parse(packet_in packet, out Headers headers, inout Metadata meta,
+         inout standard_metadata_t sm) {
+    state start {
+        packet.extract(headers.h);
+        transition accept;
+    }
+}
+
+control verifyChecksum(inout Headers headers, inout Metadata meta) { apply { } }
+control ingress(inout Headers headers, inout Metadata meta,
+                inout standard_metadata_t sm) {
+    apply {
+$0
+    }
+}
+
+control egress(inout Headers headers, inout Metadata meta,
+                inout standard_metadata_t sm) { apply { } }
+
+control computeChecksum(inout Headers headers, inout Metadata meta) { apply { } }
+
+control deparse(packet_out packet, in Headers headers) {
+    apply { packet.emit(headers.h); }
+}
+
+V1Switch(parse(), verifyChecksum(), ingress(), egress(),
+    computeChecksum(), deparse()) main;
+    )");
+
+    return FrontendTestCase::create(absl::Substitute(source, ingressSource),
+                                    CompilerOptions::FrontendVersion::P4_16, policy);
+}
+
+}  // namespace
+
+class StrengthReductionTest : public P4CTest {};
+
+struct StrengthReductionPolicy : public P4::FrontEndPolicy {
+    bool enableSubConstToAddTransform() const override { return enableSubConstToAddTransform_; }
+
+    explicit StrengthReductionPolicy(bool enableSubConstToAddTransform = true)
+        : enableSubConstToAddTransform_(enableSubConstToAddTransform) {}
+
+ private:
+    bool enableSubConstToAddTransform_ = true;
+};
+
+TEST_F(StrengthReductionTest, Default) {
+    auto test = createStrengthReductionTestCase(P4_SOURCE(R"(headers.h.f1 = headers.h.f1 - 1;)"));
+
+    ReferenceMap refMap;
+    TypeMap typeMap;
+
+    Util::SourceCodeBuilder builder;
+    ToP4 top4(builder, false);
+    test->program->apply(top4);
+
+    std::string program_string = builder.toString();
+    std::string value1 = "headers.h.f1 = headers.h.f1 + 32w4294967295";
+    std::string value2 = "headers.h.f1 = headers.h.f1 - 32w1";
+    EXPECT_FALSE(program_string.find(value1) == std::string::npos);
+    EXPECT_TRUE(program_string.find(value2) == std::string::npos);
+}
+
+TEST_F(StrengthReductionTest, DisableSubConstToAddConst) {
+    StrengthReductionPolicy policy(false);
+    auto test =
+        createStrengthReductionTestCase(P4_SOURCE(R"(headers.h.f1 = headers.h.f1 - 1;)"), &policy);
+
+    ReferenceMap refMap;
+    TypeMap typeMap;
+
+    Util::SourceCodeBuilder builder;
+    ToP4 top4(builder, false);
+    test->program->apply(top4);
+
+    std::string program_string = builder.toString();
+    std::string value1 = "headers.h.f1 = headers.h.f1 + 32w4294967295";
+    std::string value2 = "headers.h.f1 = headers.h.f1 - 32w1";
+    EXPECT_TRUE(program_string.find(value1) == std::string::npos);
+    EXPECT_FALSE(program_string.find(value2) == std::string::npos);
+}
+
+}  // namespace Test