[meta] Support not ordered enumerations (#1274)

Previously enumerations had to be defined in increasing value order, with this update enumerations names can be put together in user meaningful groups, but internally metadata will sort them and keep increasing order.
opencomputeproject · Jul 7, 2021 · 32bd93b · 32bd93b
1 parent 52e5e7f
commit 32bd93b
Show file tree

Hide file tree

Showing 6 changed files with 177 additions and 9 deletions.
diff --git a/inc/saihostif.h b/inc/saihostif.h
@@ -1010,7 +1010,7 @@ typedef enum _sai_hostif_table_entry_attr_t
      * @type sai_hostif_table_entry_type_t
      * @flags MANDATORY_ON_CREATE | CREATE_ONLY
      */
-    SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE = SAI_HOSTIF_ATTR_START,
+    SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE = SAI_HOSTIF_TABLE_ENTRY_ATTR_START,
 
     /**
      * @brief Host interface table entry match field object-id

diff --git a/meta/acronyms.txt b/meta/acronyms.txt
@@ -53,7 +53,7 @@ GRPC - GRPC Remote Procedure Call
 HW - Hardware
 I2C - Inter Integrated Circuit
 ICMP - Internet Control Message Protocol
-ICMPV6 - Internet Control Message Procotol for IPv6
+ICMPV6 - Internet Control Message Protocol for IPv6
 IEEE - Institute of Electrical and Electronics Engineers
 IFA - Inband Flow Analyzer
 IFA1 - Inband Flow Analyzer version 1

diff --git a/meta/parse.pl b/meta/parse.pl
@@ -28,6 +28,7 @@
 use strict;
 use warnings;
 use diagnostics;
+use sort 'stable'; # for enum initializers sort
 
 #use XML::Simple qw(:strict);
 use Getopt::Std;
@@ -520,6 +521,162 @@ sub ProcessDefineSection
     }
 }
 
+sub ProcessEnumInitializers
+{
+    #
+    # This function attempts to figure out enum integers values during paring
+    # time in similar way as C compiler would do. Because SAI community agreed
+    # that enum grouping is more beneficial then ordering enums, then enum
+    # values could be not sorted any more. But if we figure out integers
+    # values, we could perform stable sort at this parser level, and generate
+    # enums metadata where enum values are sorted.
+    #
+
+    my ($arr_ref, $ini_ref, $enumTypeName) = @_;
+
+    return if $enumTypeName =~ /_extensions_t$/; # ignore initializers on extensions
+
+    if (scalar(@$arr_ref) != scalar(@$ini_ref))
+    {
+        LogError "attr array not matching initializers array on $enumTypeName";
+        return;
+    }
+
+    return if grep (/<</, @$ini_ref); # skip shifted flags enum
+
+    my $previousEnumValue = -1;
+
+    my $idx = 0;
+
+    # using reference here, will cause update $ini inside initializer table
+    # reference and that's what we want
+
+    for my $ini (@$ini_ref)
+    {
+        if ($ini eq "")
+        {
+            $previousEnumValue += 1;
+
+            $ini = sprintf("0x%08x", $previousEnumValue);
+        }
+        elsif ($ini =~ /^= (0x[0-9a-f]{8})$/)
+        {
+            $previousEnumValue = hex($1);
+
+            $ini = sprintf("0x%08x", $previousEnumValue);
+        }
+        elsif ($ini =~ /^=\s+(\d+)$/)
+        {
+            $previousEnumValue = hex($1);
+
+            $ini = sprintf("0x%08x", $previousEnumValue);
+        }
+        elsif ($ini =~ /= (SAI_\w+)$/)
+        {
+            for my $i (0..$idx)
+            {
+                if ($$arr_ref[$i] eq $1)
+                {
+                    $ini = @$ini_ref[$i];
+
+                    $previousEnumValue = hex($ini);
+                    last;
+                }
+            }
+
+            LogError "initializer $ini not found on $enumTypeName before $$arr_ref[$idx]" if not $ini =~ /^0x/;
+        }
+        elsif ($ini =~ /^= (SAI_\w+) \+ (SAI_\w+)$/) # special case SAI_ACL_USER_DEFINED_FIELD_ATTR_ID_RANGE
+        {
+            # this case is in form: = (sai enum value) + (sai define)
+
+            my $first = $1;
+
+            my $val = $SAI_DEFINES{$2};
+
+            if (not defined $val)
+            {
+                LogError "$val not defined using #define directive";
+            }
+            elsif (not $val =~ /^0x[0-9a-f]+$/i)
+            {
+                LogError "$val not in hex format 0xYY";
+            }
+            else
+            {
+                for my $i (0..$idx)
+                {
+                    if ($$arr_ref[$i] eq $first)
+                    {
+                        $ini = sprintf("0x%08x", hex(@$ini_ref[$i]) + hex($val));
+
+                        $previousEnumValue = hex($ini);
+                        last;
+                    }
+                }
+
+                LogError "initializer $ini not found on $enumTypeName before $$arr_ref[$idx]" if not $ini =~ /^0x/;
+            }
+        }
+        elsif ($ini =~/^= (SAI_\w+) \+ (0x[0-9a-f]{1,8})$/)
+        {
+            my $first = $1;
+            my $val = $2;
+
+            for my $i (0..$idx)
+            {
+                if ($$arr_ref[$i] eq $first)
+                {
+                    $ini = sprintf("0x%08x", hex(@$ini_ref[$i]) + hex($val));
+
+                    $previousEnumValue = hex($ini);
+                    last;
+                }
+            }
+
+            LogError "initializer $ini not found on $enumTypeName before $$arr_ref[$idx]" if not $ini =~ /^0x/;
+        }
+        else
+        {
+            LogError "not supported initializer '$ini' on $$arr_ref[$idx], FIXME";
+        }
+
+        $idx++;
+    }
+
+    # in final form all initializers must be hex numbers 8 digits long, since
+    # they will be used in stable sort
+
+    if (scalar(grep (/^0x[0-9a-f]{8}$/, @$ini_ref)) != scalar(@$ini_ref))
+    {
+        LogError "wrong initializers on $enumTypeName: @$ini_ref";
+        return;
+    }
+
+    my $before = "@$arr_ref";
+
+    my @joined = ();
+
+    for my $idx (0..$#$arr_ref)
+    {
+        push @joined, "$$ini_ref[$idx]$$arr_ref[$idx]"; # format is: 0x00000000SAI_
+    }
+
+    my @sorted = sort { substr($a, 0, 10) cmp substr($b, 0, 10) } @joined;
+
+    s/^0x[0-9a-f]{8}SAI/SAI/i for @sorted;
+
+    my $after = "@sorted";
+
+    return if $after eq $before;
+
+    LogInfo "Need sort initalizers for $enumTypeName";
+
+    @$arr_ref = ();
+
+    push @$arr_ref, @sorted;
+}
+
 sub ProcessEnumSection
 {
     my $section = shift;
@@ -561,6 +718,7 @@ sub ProcessEnumSection
         $SAI_ENUMS{$enumtypename}{flagsenum} = ($ed =~ /\@\@flags/s) ? "true" : "false";
 
         my @arr = ();
+        my @initializers = ();
 
         $SAI_ENUMS{$enumtypename}{values} = \@arr;
 
@@ -604,8 +762,7 @@ sub ProcessEnumSection
             LogDebug "$enumtypename $enumvaluename";
 
             push@arr,$enumvaluename;
-
-            # TODO calculate each enum value using initializers
+            push@initializers,$initializer;
 
             LogWarning "Value $enumvaluename of $enumtypename is not prefixed as $enumprefix" if not $enumvaluename =~ /^$enumprefix/;
 
@@ -615,6 +772,8 @@ sub ProcessEnumSection
             }
         }
 
+        ProcessEnumInitializers(\@arr,\@initializers, $enumtypename);
+
         # TODO stable sort values based on calculated values from initializer (https://perldoc.perl.org/sort)
         # TODO add param to disable this
 

diff --git a/meta/serialize.pm b/meta/serialize.pm
@@ -851,7 +851,7 @@ sub CreateSerializeEmitMacros
     WriteSource "    if (ret < 0) {                                                 \\";
     WriteSource "        SAI_META_LOG_WARN(\"failed to serialize \" #suffix \"\");      \\";
     WriteSource "        return SAI_SERIALIZE_ERROR; }                              \\";
-    WriteSource "    buf += ret; } ";
+    WriteSource "    buf += ret; }";
     WriteSource "#define EMIT_QUOTE_CHECK(expr, suffix) {\\";
     WriteSource "    EMIT_QUOTE; EMIT_CHECK(expr, suffix); EMIT_QUOTE; }";
 }
@@ -882,7 +882,7 @@ sub CreateDeserializeEmitMacros
     WriteSource "    if (ret < 0) {                                                 \\";
     WriteSource "        SAI_META_LOG_WARN(\"failed to deserialize \" #suffix \"\");      \\";
     WriteSource "        return SAI_SERIALIZE_ERROR; }                              \\";
-    WriteSource "    buf += ret; } ";
+    WriteSource "    buf += ret; }";
     WriteSource "#define EXPECT_QUOTE_CHECK(expr, suffix) {\\";
     WriteSource "    EXPECT_QUOTE; EXPECT_CHECK(expr, suffix); EXPECT_QUOTE; }";
 }

diff --git a/meta/style.pm b/meta/style.pm
@@ -951,14 +951,15 @@ sub CheckHeadersStyle
             {
                 my $init = $2;
 
-                if ($init =~ m!^(0x\w+|SAI_\w+|SAI_\w+ \+ SAI_\w+|\d+|\(?\d+ << \d+\)?),?\s*(/\*\*.*\*/)?$!)
+                if ($init =~ m!^(0x\w+|SAI_\w+|SAI_\w+ \+ 0x[0-9a-f]{1,8}|SAI_\w+ \+ SAI_\w+|\d+|\(?\d+ << \d+\)?),?\s*(/\*\*.*\*/)?$!)
                 {
                     # supported initializers for enum:
                     # - 0x00000000 (hexadecimal number)
                     # - 0 (decimal number)
                     # - SAI_... (other SAI enum)
                     # - n << m (flags shifted)
                     # - SAI_.. + SAI_.. (sum of SAI enums)
+                    # - SAI_.. + 0x00 (sum of SAI and hexadecimal number)
                 }
                 else
                 {

diff --git a/meta/utils.pm b/meta/utils.pm
@@ -61,7 +61,11 @@ sub WriteHeader
 
     my $ident = GetIdent($content);
 
-    $HEADER_CONTENT .= $ident . $content . "\n";
+    my $line = $ident . $content . "\n";
+
+    $line = "\n" if $content eq "";
+
+    $HEADER_CONTENT .= $line;
 }
 
 sub WriteSource
@@ -70,7 +74,11 @@ sub WriteSource
 
     my $ident = GetIdent($content);
 
-    $SOURCE_CONTENT .= $ident . $content . "\n";
+    my $line = $ident . $content . "\n";
+
+    $line = "\n" if $content eq "";
+
+    $SOURCE_CONTENT .= $line;
 }
 
 sub WriteTest