Fixup/cleanup IR::Type::width_bits() (p4lang#4858)

- this method is used by backends to get the size of a type, and should only be used for Types for which the concept of `size in bits' makes sense Signed-off-by: Chris Dodd <[email protected]>
nus-sys · Aug 10, 2024 · 0e2a7d6 · 0e2a7d6
1 parent 360d2a2
commit 0e2a7d6
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 6 deletions.
diff --git a/backends/bmv2/common/control.h b/backends/bmv2/common/control.h
@@ -449,7 +449,13 @@ class ControlConverter : public Inspector {
             for (auto k : keyset->components) {
                 auto key = new Util::JsonObject();
                 auto tableKey = table->getKey()->keyElements.at(keyIndex);
-                auto keyWidth = tableKey->expression->type->width_bits();
+                int keyWidth = 0;
+                if (tableKey->expression->type->is<IR::Type_Error>()) {
+                    // error type doesn't have a width, and will fail below, checking the key
+                    // expression k, so it doesn't matter what keyWidth is.
+                } else {
+                    keyWidth = tableKey->expression->type->width_bits();
+                }
                 auto k8 = ROUNDUP(keyWidth, 8);
                 auto matchType = getKeyMatchType(tableKey);
                 // Table key fields with match_kind optional will be

diff --git a/backends/bmv2/psa_switch/psaSwitch.h b/backends/bmv2/psa_switch/psaSwitch.h
@@ -39,7 +39,9 @@ class PsaSwitchExpressionConverter : public ExpressionConverter {
             auto jsn = new Util::JsonObject();
             jsn->emplace("name", param->toString());
             jsn->emplace("type", "hexstr");
-            auto bitwidth = param->type->width_bits();
+            // FIXME -- how big is a PSA_CounterType_t?  Being an enum type, we can't
+            // sensibly call param->width_bits() here.
+            auto bitwidth = 0;
 
             // encode the counter type from enum -> int
             if (fieldName == "BYTES") {

diff --git a/backends/p4tools/common/lib/variables.cpp b/backends/p4tools/common/lib/variables.cpp
@@ -34,7 +34,7 @@ const IR::TaintExpression *getTaintExpression(const IR::Type *type) {
     }
     // Only cache bits with width lower than 16 bit to restrict the size of the cache.
     const auto *tb = type->to<IR::Type_Bits>();
-    if (type->width_bits() > 16 || tb == nullptr) {
+    if (tb == nullptr || type->width_bits() > 16) {
         return new IR::TaintExpression(type);
     }
     // Taint expressions are interned. Keys in the intern map is the signedness and width of the

diff --git a/backends/p4tools/modules/testgen/test/lib/taint.cpp b/backends/p4tools/modules/testgen/test/lib/taint.cpp
@@ -285,7 +285,8 @@ TEST_F(TaintTest, Taint09) {
 
     // (32w0 ++ Taint64b ++ 32w0) & 128w0
     // The bitwise and should not have any effect on taint.
-    const auto *taint128bMiddleQ2 = new IR::BAnd(taint128bMiddleQ, new IR::Constant(128));
+    const auto *taint128bMiddleQ2 =
+        new IR::BAnd(taint128bMiddleQ, IR::Constant::get(IR::Type::Bits::get(128), 0));
     ASSERT_TRUE(!Taint::hasTaint(new IR::Slice(taint128bMiddleQ2, 127, 96)));
     ASSERT_TRUE(Taint::hasTaint(new IR::Slice(taint128bMiddleQ2, 95, 32)));
     ASSERT_TRUE(!Taint::hasTaint(new IR::Slice(taint128bMiddleQ2, 31, 0)));

diff --git a/ir/base.def b/ir/base.def
@@ -36,7 +36,7 @@ abstract Type {
     typedef Type_Void           Void;
 #end
     /// Well-defined only for types with fixed width
-    virtual int width_bits() const { return 0; }
+    virtual int width_bits() const { BUG("width_bits() on type with unknown size: %1%", this); }
     /// When possible returns the corresponding type that can be inserted
     /// in a P4 program; may return a Type_Name
     virtual const Type* getP4Type() const = 0;

diff --git a/ir/expression.def b/ir/expression.def
@@ -321,7 +321,7 @@ class TypeNameExpression : Expression {
     dbprint{ out << typeName; }
     toString { return typeName->toString(); }
     validate { BUG_CHECK(typeName->is<Type_Name>() || typeName->is<Type_Specialized>(),
-                         "%1 unexpected type in TypeNameExpression", typeName); }
+                         "%1% unexpected type in TypeNameExpression", typeName); }
 }
 
 class Slice : Operation_Ternary {

diff --git a/ir/type.def b/ir/type.def
@@ -182,6 +182,7 @@ class Type_Varbits : Type_Base {
     static Type_Varbits get();
     toString{ return "varbit<"_cs + Util::toString(size) + ">"_cs; }
     dbprint { out << "varbit<" << size << ">"; }
+    int width_bits() const override { return size; }
 }
 
 class Parameter : Declaration, IAnnotated {
@@ -268,6 +269,7 @@ class Type_InfInt : Type, ITypeVar {
         return true; /* ignore declid */
     }
     const Type* getP4Type() const override { return this; }
+    int width_bits() const override { return 0; }
 }
 
 class Type_Dontcare : Type_Base {
@@ -583,6 +585,7 @@ class Type_Stack : Type_Indexed, Type {
     static const cstring pop_front;
     const Type* getP4Type() const override
     { return new IR::Type_Stack(srcInfo, elementType->getP4Type(), size); }
+    int width_bits() const override { return getSize() * elementType->width_bits(); }
 }
 
 /** Given a declaration
@@ -680,6 +683,7 @@ class Type_SerEnum : Type_Declaration, ISimpleNamespace, IAnnotated {
         return members.getDeclaration(name); }
 #nodbprint
     validate{ members.check_null(); }
+    int width_bits() const override { return type->width_bits(); }
 }
 
 class Type_Table : Type, IApply {