Gate SIM_register_typed_attribute behind a compat feature

Author: lwaern-intel

RELEASENOTES.docu

@@ -1776,4 +1776,12 @@ extern typedef struct { } my_type_t;</pre> </add-note></build-id>
       the <tt>dead_dml_methods</tt> library, which caused it to never
       detect methods as dead if they occur directly after
       a <tt>footer</tt> block.</add-note></build-id>
+  <build-id _6="next" _7="next"><add-note>DMLC's use of the legacy Simics API
+      for attribute registration is now controlled by the compatibility feature
+      <tt>legacy_attributes</tt>. This feature can be disabled by passing
+      <tt>--no-compat=legacy_attributes</tt> to DMLC, in which case the modern
+      API will be used instead, which does not support the use of dictionary
+      attribute values <bug number="SIMICS-22406"/>.
+      <tt>legacy_attributes</tt> will always be disabled with Simics API version
+      8 or above.</add-note></build-id>
 </rn>

include/simics/dmllib.h

@@ -8,6 +8,10 @@
 #ifndef SIMICS_DMLLIB_H
 #define SIMICS_DMLLIB_H
 
+#ifndef DML_LEGACY_ATTRS
+#define DML_LEGACY_ATTRS 0
+#endif
+
 #include <stddef.h>
 
 #include <simics/base/event.h>
@@ -1819,6 +1823,53 @@ typedef attr_value_t (*_get_attr_t)(conf_object_t *, lang_void *);
 typedef set_error_t (*_set_attr_t)(conf_object_t *, attr_value_t *,
                                    lang_void *);
 
+#if DML_LEGACY_ATTRS
+typedef struct {
+    _get_attr_t get_attr;
+    _set_attr_t set_attr;
+    lang_void   *user_data_get;
+    lang_void   *user_data_set;
+} _dml_legacy_attr_info_t;
+
+UNUSED static attr_value_t
+_DML_legacy_attr_get_trampoline(lang_void *_info, conf_object_t *obj,
+                                attr_value_t *_idx) {
+    const _dml_legacy_attr_info_t *info = (_dml_legacy_attr_info_t *)_info;
+    return info->get_attr(obj, info->user_data_get);
+}
+
+UNUSED static set_error_t
+_DML_legacy_attr_set_trampoline(lang_void *_info, conf_object_t *obj,
+                                attr_value_t *out, attr_value_t *_idx) {
+    const _dml_legacy_attr_info_t *info = (_dml_legacy_attr_info_t *)_info;
+    return info->set_attr(obj, out, info->user_data_set);
+}
+
+UNUSED static void
+_DML_register_attribute(
+    conf_class_t *NOTNULL cls, const char *NOTNULL name,
+    _get_attr_t get_attr, lang_void *user_data_get,
+    _set_attr_t set_attr, lang_void *user_data_set,
+    attr_attr_t attr, const char *type, const char *desc) {
+    _dml_legacy_attr_info_t *info = MM_MALLOC(1, _dml_legacy_attr_info_t);
+    info->get_attr = get_attr;
+    info->set_attr = set_attr;
+    info->user_data_get = user_data_get;
+    info->user_data_set = user_data_set;
+
+    SIM_register_typed_attribute(cls, name,
+                                 get_attr ? _DML_legacy_attr_get_trampoline
+                                          : NULL,
+                                 get_attr ? (lang_void *)info : NULL,
+                                 set_attr ? _DML_legacy_attr_set_trampoline
+                                          : NULL,
+                                 set_attr ? (lang_void *)info : NULL,
+                                 attr, type, NULL, desc);
+}
+#else
+#define _DML_register_attribute SIM_register_attribute_with_user_data
+#endif
+
 UNUSED static void
 _DML_register_hook_attribute(conf_class_t *cls, const char *attrname,
                              _get_attr_t getter, _set_attr_t setter,
@@ -1984,9 +2035,8 @@ _register_port_attr_no_aux(conf_class_t *portcls, const char *attrname,
                            attr_attr_t attr, const char *type,
                            const char *desc)
 {
-        SIM_register_attribute_with_user_data(
-                portcls, attrname, getter, NULL, setter, NULL,
-                attr, type, desc);
+        _DML_register_attribute(portcls, attrname, getter, NULL, setter, NULL,
+                                attr, type, desc);
 }
 
 // port_obj_offset is the offset within the device struct of a pointer to the
@@ -2015,7 +2065,8 @@ _register_port_legacy_proxy_attr(conf_class_t *devcls, conf_class_t *portcls,
         strbuf_t proxy_desc = sb_newf(
                 "Proxy attribute for %s.%s.%s",
                 is_bank ? "bank" : "port", portname, attrname);
-        SIM_register_attribute_with_user_data(
+
+        _DML_register_attribute(
                 devcls, name,
                 getter ? _get_legacy_proxy_attr : NULL, data,
                 setter ? _set_legacy_proxy_attr : NULL, data,
@@ -2116,7 +2167,7 @@ _register_port_array_legacy_proxy_attr(
                                       is_bank ? "bank" : "port",
                                       portname, attrname);
         strbuf_t proxy_type = sb_newf("[%s{%d}]", type, array_size);
-        SIM_register_attribute_with_user_data(
+        _DML_register_attribute(
                 devcls, name,
                 getter ? _get_legacy_proxy_array_attr : NULL, data,
                 setter ? _set_legacy_proxy_array_attr : NULL, data,
@@ -3414,7 +3465,7 @@ UNUSED static void _DML_register_attributes(
             MM_FREE(tmp_type);
         }
         conf_class_t *parent_obj_class = attr_info.parent_obj_class;
-        SIM_register_attribute_with_user_data(
+        _DML_register_attribute(
             parent_obj_class ? parent_obj_class : dev_class,
             attr_info.name,
             attr_info.readable ? parent_obj_class ? get_portobj_attr : get_attr

lib/1.2/dml-builtins.dml

@@ -215,6 +215,7 @@ template device {
     parameter _compat_io_memory auto;
     parameter _compat_shared_logs_on_device auto;
     parameter _compat_suppress_WLOGMIXUP auto;
+    parameter _compat_legacy_attributes auto;
     parameter _compat_dml12_inline auto;
     parameter _compat_dml12_not auto;
     parameter _compat_dml12_goto auto;

lib/1.4/dml-builtins.dml

@@ -561,6 +561,7 @@ template device {
     param _compat_io_memory auto;
     param _compat_shared_logs_on_device auto;
     param _compat_suppress_WLOGMIXUP auto;
+    param _compat_legacy_attributes auto;
     param _compat_dml12_inline auto;
     param _compat_dml12_not auto;
     param _compat_dml12_goto auto;

py/dml/c_backend.py

@@ -260,6 +260,8 @@ def generate_hfile(device, headers, filename):
     out("/* Generated by dmlc, do not edit! */\n\n")
     emit_guard_start(filename)
     out('#define DML_PREFIX(x) '+crep.cname(device)+'_##x\n\n')
+    legacy_attrs = int(compat.legacy_attributes in dml.globals.enabled_compat)
+    out(f'#define DML_LEGACY_ATTRS {legacy_attrs}\n')
 
     with allow_linemarks():
         for c in headers:
@@ -624,7 +626,7 @@ def generate_attribute_common(initcode, node, port, dimsizes, prefix,
                    attr_flag, attr_type, doc.read()))
     else:
         initcode.out(
-            f'SIM_register_attribute_with_user_data(class, "{attrname}", '
+            f'_DML_register_attribute(class, "{attrname}", '
             + f'{getter}, NULL, {setter}, NULL, {attr_flag}, "{attr_type}", '
             + f'{doc.read()});\n')
 

py/dml/compat.py

@@ -198,6 +198,23 @@ class suppress_WLOGMIXUP(CompatFeature):
     short = "Suppress the warning 'WLOGMIXUP' by default"
     last_api_version = api_6
 
+@feature
+class legacy_attributes(CompatFeature):
+    '''This compatibility feature makes DMLC register all attributes using the
+    legacy `SIM_register_typed_attribute` API function instead of the modern
+    `SIM_register_attribute` family of API functions.
+
+    Disabling this feature will make the dictionary attribute type ("D" in type
+    strings) to become unsupported, and any usage of it rejected by Simics.
+    Thus, when migrating away from this feature, any attribute of the model
+    that leverages dictionary values should be changed to leverage a different
+    representation. In general, any dictionary can instead be represented by a
+    list of two-element lists, e.g. <code>[[<em>X</em>,<em>Y</em>]*]</code>,
+    where _X_ describes the type of keys, and _Y_ describes the type of
+    values.'''
+    short = ("Use legacy attribute registration, which supports use of the "
+             + "dictionary type ('D')")
+    last_api_version = api_7
 
 @feature
 class dml12_inline(CompatFeature):

test/1.4/legacy/T_legacy_attributes_disabled.dml

@@ -0,0 +1,12 @@
+/*
+  © 2024 Intel Corporation
+  SPDX-License-Identifier: MPL-2.0
+*/
+dml 1.4;
+device test;
+
+// NOTE: this test relies on proxy attributes
+/// DMLC-FLAG --simics-api=7
+/// DMLC-FLAG --no-compat=legacy_attributes
+
+import "legacy_attributes.dml";

test/1.4/legacy/T_legacy_attributes_enabled.dml

@@ -0,0 +1,22 @@
+/*
+  © 2024 Intel Corporation
+  SPDX-License-Identifier: MPL-2.0
+*/
+dml 1.4;
+device test;
+
+/// DMLC-FLAG --simics-api=7
+
+import "legacy_attributes.dml";
+
+attribute dict_attr is read_only_attr "some dict attr" {
+    param type = "D";
+    method get() -> (attr_value_t) {
+        return SIM_alloc_attr_dict(0);
+    }
+}
+
+method init() {
+    default();
+    assert SIM_attr_is_dict((SIM_get_attribute(dev.obj, "dict_attr")));
+}

test/1.4/legacy/legacy_attributes.dml

@@ -0,0 +1,43 @@
+/*
+  © 2024 Intel Corporation
+  SPDX-License-Identifier: MPL-2.0
+*/
+dml 1.4;
+
+import "simics/simulator/conf-object.dml";
+
+saved int s;
+hook() h;
+
+attribute a is uint64_attr;
+
+bank b {
+    saved int s;
+    hook() h;
+    attribute a is uint64_attr;
+}
+
+method init() default {
+    local attr_value_t val = SIM_make_attr_uint64(0xC0FFEE);
+    #foreach r in ([dev, b]) {
+        local conf_class_t *cls = SIM_object_class(r.obj);
+        assert (SIM_get_attribute_attributes(cls, "s") & Sim_Attr_Legacy)
+               == 0;
+        assert (SIM_get_attribute_attributes(cls, "h") & Sim_Attr_Legacy)
+               == 0;
+
+        assert ((SIM_get_attribute_attributes(cls, "a") & Sim_Attr_Legacy)
+                != 0) == _compat_legacy_attributes;
+
+        SIM_set_attribute(r.obj, "a", &val);
+        assert SIM_attr_integer(SIM_get_attribute(r.obj, "a"))
+            == 0xC0FFEE;
+    }
+    assert ((SIM_get_attribute_attributes(SIM_object_class(dev.obj), "b_a")
+             & Sim_Attr_Legacy) != 0) == _compat_legacy_attributes;
+
+    val = SIM_make_attr_uint64(0xDEADBEEF);
+    SIM_set_attribute(dev.obj, "b_a", &val);
+    assert SIM_attr_integer(SIM_get_attribute(dev.obj, "b_a")) == 0xDEADBEEF;
+    assert SIM_attr_integer(SIM_get_attribute(b.obj, "a")) == 0xDEADBEEF;
+}

test/1.4/structure/T_portobj.dml

@@ -6,6 +6,8 @@ dml 1.4;
 
 device test;
 
+/// DMLC-FLAG --no-compat=legacy_attributes
+
 import "simics/simulator/conf-object.dml";
 
 bank b[i<2] {