Fix float() ordering problem for DDI tables (#488)

+ Add class information for new graph APIs to ensure
  ordinals are correctly used
+ Fix for bug that used float() to order based on versions.
  Hence causing 1.17 to be less than 1.4.
+ Permanently simplify version ordering within the spec
+ Use Ordinal to return zet ddi table to original order, this fix
  will ensure despite the bad order due to past bugs. The order
  is harmless but it needs to stay in this manner.

Signed-off-by: Russell McGuire <russell.w.mcguire@intel.com>

Author: rwmcguir

scripts/core/graph.yml

@@ -398,4 +398,28 @@ details:
 params:
     - type: $x_graph_handle_t
       name: hGraph
-      desc: "[in][release] handle of the graph to destroy"
+      desc: "[in][release] handle of the graph to destroy"
+--- #--------------------------------------------------------------------------
+type: class
+desc: "C++ wrapper for a recorded graph object"
+name: $xGraph
+owner: $xContext
+members:
+    - type: $x_graph_handle_t
+      name: handle
+      desc: "[in] handle of graph object"
+    - type: $xContext*
+      name: pContext
+      desc: "[in] pointer to owner object"
+--- #--------------------------------------------------------------------------
+type: class
+desc: "C++ wrapper for an executable graph object"
+name: $xExecutableGraph
+owner: $xGraph
+members:
+    - type: $x_executable_graph_handle_t
+      name: handle
+      desc: "[in] handle of executable graph object"
+    - type: $xGraph*
+      name: pGraph
+      desc: "[in] pointer to owner object"

scripts/parse_specs.py

@@ -11,7 +11,7 @@
 import json
 import yaml
 import copy
-from templates.helper import param_traits, type_traits, value_traits, get_tag
+from templates.helper import param_traits, type_traits, value_traits, get_tag, version_key
 
 default_version = "1.0"
 all_versions = ["1.0", "1.1", "1.2", "1.3", "1.4", "1.5", "1.6", "1.7", "1.8", "1.9", "1.10", "2.0"]
@@ -526,120 +526,22 @@ def _validate_struct_enum_mapping(specs, tags):
 """
     filters object by version
 """
+# Version comparisons route through helper.version_key (single source of truth
+# for major/minor decomposition). Do NOT reintroduce float(version) here.
 def _version_compare_greater(a, b):
-    a_major = int(a.split('.')[0])
-    a_minor = int(a.split('.')[1])
-
-    b_major = int(b.split('.')[0])
-    b_minor = int(b.split('.')[1])
-
-    if (a_major > b_major):
-        # print("DEBUG: greater(%d.%d, %d.%d) -> True (major)" % (a_major, a_minor, b_major, b_minor))
-        return True
-
-    if (a_major < b_major):
-        # print("DEBUG: greater(%d.%d, %d.%d) -> False (major)" % (a_major, a_minor, b_major, b_minor))
-        return False
-
-    # a_major == b_major
-
-    if (a_minor > b_minor):
-        # print("DEBUG: greater(%d.%d, %d.%d) -> True (minor)" % (a_major, a_minor, b_major, b_minor))
-        return True
-
-    # a_minor <= b_minor
-
-    # print("DEBUG: greater(%d.%d, %d.%d) -> False (minor)" % (a_major, a_minor, b_major, b_minor))
-    return False
+    return version_key(a) > version_key(b)
 
 def _version_compare_equal(a, b):
-    a_major = int(a.split('.')[0])
-    a_minor = int(a.split('.')[1])
-
-    b_major = int(b.split('.')[0])
-    b_minor = int(b.split('.')[1])
-
-    ret_val = ((a_major == b_major) and (a_minor == b_minor))
-    # print("DEBUG: equal(%d.%d, %d.%d) -> %s" % (a_major, a_minor, b_major, b_minor, ret_val))
-    return ret_val
+    return version_key(a) == version_key(b)
 
 def _version_compare_less(a, b):
-    a_major = int(a.split('.')[0])
-    a_minor = int(a.split('.')[1])
-
-    b_major = int(b.split('.')[0])
-    b_minor = int(b.split('.')[1])
-
-    if a_major > b_major:
-        # print("DEBUG: less(%d.%d, %d.%d) -> False (major)" % (a_major, a_minor, b_major, b_minor))
-        return False
-
-    if a_major < b_major:
-        # print("DEBUG: less(%d.%d, %d.%d) -> True (major)" % (a_major, a_minor, b_major, b_minor))
-        return True
-
-    # a_major == b_major
-
-    if a_minor < b_minor:
-        # print("DEBUG: less(%d.%d, %d.%d) -> True (minor)" % (a_major, a_minor, b_major, b_minor))
-        return True
-
-    # a_minor >= b_minor
-
-    # print("DEBUG: less(%d.%d, %d.%d) -> False (minor)" % (a_major, a_minor, b_major, b_minor))
-    return False
+    return version_key(a) < version_key(b)
 
 def _version_compare_lequal(a, b):
-    a_major = int(a.split('.')[0])
-    a_minor = int(a.split('.')[1])
-
-    b_major = int(b.split('.')[0])
-    b_minor = int(b.split('.')[1])
-
-    if a_major > b_major:
-        # print("DEBUG: lequal(%d.%d, %d.%d) -> False (major)" % (a_major, a_minor, b_major, b_minor))
-        return False
-
-    if a_major < b_major:
-        # print("DEBUG: lequal(%d.%d, %d.%d) -> True (major)" % (a_major, a_minor, b_major, b_minor))
-        return True
-
-    # a_major == b_major
-
-    if a_minor <= b_minor:
-        # print("DEBUG: lequal(%d.%d, %d.%d) -> True (minor)" % (a_major, a_minor, b_major, b_minor))
-        return True
-
-    # a_minor > b_minor
-
-    # print("DEBUG: lequal(%d.%d, %d.%d) -> False (minor)" % (a_major, a_minor, b_major, b_minor))
-    return False
+    return version_key(a) <= version_key(b)
 
 def _version_compare_gequal(a, b):
-    a_major = int(a.split('.')[0])
-    a_minor = int(a.split('.')[1])
-
-    b_major = int(b.split('.')[0])
-    b_minor = int(b.split('.')[1])
-
-    if a_major > b_major:
-        # print("DEBUG: gequal(%d.%d, %d.%d) -> True (major)" % (a_major, a_minor, b_major, b_minor))
-        return True
-
-    if a_major < b_major:
-        # print("DEBUG: gequal(%d.%d, %d.%d) -> False (major)" % (a_major, a_minor, b_major, b_minor))
-        return False
-
-    # a_major == b_major
-
-    if a_minor >= b_minor:
-        # print("DEBUG: gequal(%d.%d, %d.%d) -> True (minor)" % (a_major, a_minor, b_major, b_minor))
-        return True
-
-    # a_minor < b_minor
-
-    # print("DEBUG: gequal(%d.%d, %d.%d) -> False (minor)" % (a_major, a_minor, b_major, b_minor))
-    return False
+    return version_key(a) >= version_key(b)
 
 def _filter_version(d, max_ver):
     ver = d.get('version', default_version)
@@ -662,7 +564,7 @@ def __filter_detail(det):
             detail = None
             for k, v in det.items():
                 try:
-                    version = float(k)
+                    version_key(k)
                 except:
                     return det
                 if _version_compare_lequal(k,  max_ver):
@@ -1122,7 +1024,12 @@ def parse(section, version, tags, meta, ref):
             # extract header from objects
             if re.match(r"header", d['type']):
                 header = d
-                header['ordinal'] = int(int(header.get('ordinal',"1000")) * float(header.get('version',"1.0")))
+                # Header ordinal drives DDI table/class order (and the global
+                # spec emission sort below). version_key() decomposes major/minor
+                # so e.g. "1.17" sorts after "1.4"; a naive float(version) would
+                # treat "1.17" as 1.17 < "1.4"==1.4 and misplace sub-tables,
+                # breaking N-1 ABI. See helper.version_key.
+                header['ordinal'] = int(int(header.get('ordinal',"1000")) * version_key(header.get('version',"1.0")))
                 header['ordinal'] *= 1000 if re.match(r"extension", header.get('desc',"").lower()) else 1
                 header['ordinal'] *= 1000 if re.match(r"experimental", header.get('desc',"").lower()) else 1
 

scripts/templates/helper.py

@@ -6,6 +6,34 @@
 """
 import re
 
+"""
+    Single source of truth for version ordering.
+
+    Maps a "major.minor" version string to a sortable/comparable integer
+    ("1.10" -> 10010, "1.4" -> 10004) via major/minor decomposition.
+
+    NEVER use float(version) for version comparison or sorting:
+    float("1.10") == 1.1, which is *less* than float("1.4") == 1.4, so any
+    two-digit minor version silently sorts before lower real versions. That
+    misplaces DDI sub-tables in the container struct and breaks N-1 ABI.
+"""
+_VERSION_MINOR_BASE = 10000   # major*BASE + minor; a minor >= BASE aliases (major+1).0
+
+def version_key(version):
+    major, _, minor = str(version).partition(".")
+    minor = int(minor) if minor else 0
+    # Loud failure beats a silent wrap that would reorder a DDI table and break ABI.
+    assert 0 <= minor < _VERSION_MINOR_BASE, \
+        "minor version %d exceeds version_key headroom (%d); raise _VERSION_MINOR_BASE" % (minor, _VERSION_MINOR_BASE)
+    return int(major) * _VERSION_MINOR_BASE + minor
+
+"""
+    Stable ordering key for a function/object within a DDI table: order by
+    version first, then by the object's explicit 'ordinal' (default 100).
+"""
+def function_sort_key(obj):
+    return (version_key(obj.get('version', "1.0")), int(obj.get('ordinal', "100")))
+
 """
     Extracts traits from a spec object
 """
@@ -913,14 +941,14 @@ def get_class_function_objs(specs, cname, version = None, includeExt = False):
             is_function = obj_traits.is_function(obj)
             match_cls = cname == obj_traits.class_name(obj)
             if is_function and match_cls:
-                if version is None or (float(obj.get('version',"1.0")) <= version):
+                if version is None or (version_key(obj.get('version',"1.0")) <= version_key(version)):
                     if obj_traits.is_extension(obj):
                         if includeExt: 
                             objects.append(obj)
                     else:
                         objects.append(obj)
 
-    return sorted(objects, key=lambda obj: (float(obj.get('version',"1.0").split(".")[0]) * 10000000) + (float(obj.get('version',"1.0").split(".")[1])*10000) + int(obj.get('ordinal',"100")))
+    return sorted(objects, key=function_sort_key)
 
 """
 Public:
@@ -938,8 +966,8 @@ def get_class_function_objs_exp(specs, cname):
                     exp_objects.append(obj)
                 else:
                     objects.append(obj)
-    objects = sorted(objects, key=lambda obj: (float(obj.get('version',"1.0").split(".")[0]) * 10000000) + (float(obj.get('version',"1.0").split(".")[1]) * 10000) + int(obj.get('ordinal',"100")))
-    exp_objects = sorted(exp_objects, key=lambda obj: (float(obj.get('version',"1.0").split(".")[0]) * 10000000) + (float(obj.get('version',"1.0").split(".")[1])* 10000) + int(obj.get('ordinal',"100")))              
+    objects = sorted(objects, key=function_sort_key)
+    exp_objects = sorted(exp_objects, key=function_sort_key)
     return objects, exp_objects
 
 """ 
@@ -1035,7 +1063,7 @@ def get_pfntables(specs, meta, namespace, tags):
 def get_pfncbtables(specs, meta, namespace, tags):
     tables = []
     for cname in sorted(meta['class'], key=lambda x: meta['class'][x]['ordinal']):
-        objs = get_class_function_objs(specs, cname, 1.0)
+        objs = get_class_function_objs(specs, cname, "1.0")
         if len(objs) > 0:
             name = get_table_name(namespace, tags, {'class': cname})
             table = "%s_%s_callbacks_t"%(namespace, _camel_to_snake(name))

scripts/tools/concurrentMetricGroup.yml

@@ -30,6 +30,7 @@ desc: "Get sets of metric groups which could be collected concurrently."
 version: "1.10"
 class: $tDevice
 name: GetConcurrentMetricGroupsExp
+ordinal: "1"
 decl: static
 details:
     - "Re-arrange the input metric groups to provide sets of concurrent metric groups."

scripts/tools/metricProgrammable.yml

@@ -446,6 +446,7 @@ desc: "Create multiple metric group handles from metric handles."
 version: "1.10"
 class: $tDevice
 name: CreateMetricGroupsFromMetricsExp
+ordinal: "2"
 decl: static
 details:
     - "Creates multiple metric groups from metrics which were created using $tMetricCreateFromProgrammableExp2()."