From c3171ad9d553731a742a4e48ddb99b98bdc53650 Mon Sep 17 00:00:00 2001 From: Love Waern Date: Fri, 14 Jun 2024 13:53:20 +0200 Subject: [PATCH 1/2] Add a warning for large unrolled loops --- RELEASENOTES.docu | 3 ++ py/dml/codegen.py | 11 +++++ py/dml/messages.py | 23 +++++++++ test/1.2/errors/T_WBIGUNROLL.dml | 83 ++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+) create mode 100644 test/1.2/errors/T_WBIGUNROLL.dml diff --git a/RELEASENOTES.docu b/RELEASENOTES.docu index beda591ab..3f2b965b2 100644 --- a/RELEASENOTES.docu +++ b/RELEASENOTES.docu @@ -1795,4 +1795,7 @@ extern typedef struct { } my_type_t; Multiple `extern` declarations for the same identifier are now allowed as long as they all declare the same type for the identifier . + Added the warning WBIGUNROLL + which is emitted when a #select or #foreach statement is + compiled to a large unrolled loop. diff --git a/py/dml/codegen.py b/py/dml/codegen.py index 861ebcc22..8b05a6240 100644 --- a/py/dml/codegen.py +++ b/py/dml/codegen.py @@ -3031,12 +3031,19 @@ def stmt_select(stmt, location, scope): else_dead = True break + iterations = len(clauses) if else_dead: (last_cond, last_stmt) = clauses.pop(-1) assert last_cond.constant and last_cond.value if_chain = last_stmt else: if_chain = codegen_statement(else_ast, location, scope) + if iterations > WBIGUNROLL.limit and isinstance(lst, List): + report(WBIGUNROLL(stmt.site, + '#'*(stmt.site.dml_version() != (1, 2)) + + 'select', + iterations)) + for (cond, stmt) in reversed(clauses): if_chain = mkIf(cond.site, cond, stmt, if_chain) return [if_chain] @@ -3162,6 +3169,10 @@ def foreach_constant_list(site, itername, lst, statement, location, scope): stmt) spec.append(mkCompound(site, decls + [stmt])) + if len(spec) > WBIGUNROLL.limit and isinstance(lst, List): + report(WBIGUNROLL(site, + '#'*(site.dml_version() != (1, 2)) + 'foreach', + len(spec))) return [mkUnrolledLoop(site, spec, context.label if context.used else None)] diff --git a/py/dml/messages.py b/py/dml/messages.py index 8d7b9cc21..52d9f0672 100644 --- a/py/dml/messages.py +++ b/py/dml/messages.py @@ -2291,6 +2291,29 @@ class WHOOKSEND(DMLWarning): + "Declarations section in the DML 1.4 reference manual for " + "information about the differences between 'send' and 'send_now'") +class WBIGUNROLL(DMLWarning): + limit = 64 + __doc__ = f""" + A `#select` or `#foreach` statement was specified which caused DMLC to + study the body and generate duplicated C code for it a large number of + times (more than {limit} times.) This can dramatically increase both DMLC + and GCC compile times and code size, if not crash DMLC outright. + + To address this, you have two options: + * Ensure most iterations can be entirely eliminated at compile-time by + DMLC. For `#select`, this can be done by ensuring that the `where` check + will be a constant value (e.g. a constant equality) for most if not all + items of the specified list. For `#foreach`, encase the body in an `#if` + check that is false for most items of the specified list. + * Don't use `#select` or `#foreach`. Represent the specified compile-time + list instead as a `session` array or an `extern`ed C array, and iterate + through it using traditional `for` loops. For more information, see [the + various answers to this Stack Overflow question.](https://stackoverflow.com/questions/75073681/cannot-use-variable-index-in-a-constant-list) + """ + fmt = ("This '%s' statement compiles to an unrolled loop where the loop " + + "body is studied and generated %s times. This may dramatically " + + "increase both DMLC and GCC compilation times, if not crash DMLC " + + "outright!") class PSHA1(PortingMessage): """The `port-dml` script requires that the DML file has not been diff --git a/test/1.2/errors/T_WBIGUNROLL.dml b/test/1.2/errors/T_WBIGUNROLL.dml new file mode 100644 index 000000000..81d63c574 --- /dev/null +++ b/test/1.2/errors/T_WBIGUNROLL.dml @@ -0,0 +1,83 @@ +/* + © 2023 Intel Corporation + SPDX-License-Identifier: MPL-2.0 +*/ +dml 1.2; + +device test; + +/// COMPILE-ONLY +/// NO-CC + +data int zero = 0; + +parameter zeroes_64 = [$zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero]; + +// Sixtyfourth zero is constant +parameter zeroes_65 = [$zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, $zero, + $zero, $zero, $zero, $zero, $zero, $zero, $zero, 0, + $zero]; + +bank b { + register regs[i in 0..64] size 4 @ undefined; +} + +method init { + local int nonconstant; + + // no warning + foreach x in ($zeroes_64) assert true; + // The else branch is not considered an iteration + select x in ($zeroes_64) where (x == nonconstant) { + assert true; + } else assert true; + + /// WARNING WBIGUNROLL + foreach x in ($zeroes_65) assert true; + /// WARNING WBIGUNROLL + select x in ($zeroes_65) where (x == nonconstant) { + assert true; + } else assert true; + + // no warning + foreach x in ($zeroes_65) { + // As sixtyfourth bit is constant 0, DML 1.2 semantics eliminates this + // if, and subsequently causes that iteration to be omitted from + // codegen entirely; reducing the total count to 64 + if (x != 0) assert true; + } + + // As sixtyfourth bit is constant 0, select can cut short at it, reducing + // the total number of iterations to 64 + select x in ($zeroes_65) where (x == 0) { + assert true; + } else assert true; + + // As sixtyfourth bit is constant 0, select can omit the check and thus + // iteration for it, reducing the total number of iterations to 64 + select x in ($zeroes_65) where (x != 0) { + assert true; + } else assert true; + + + // WBIGUNROLL is only reported for compile-time lists defined via list + // syntax, not the object lists generated by DMLC + // no warning + foreach x in ($b.unmapped_registers) assert true; + select x in ($b.unmapped_registers) where (x == nonconstant) { + assert true; + } else assert true; +} From d6cbc1ab7c790f7f397e6fdab1bdc3378519f174 Mon Sep 17 00:00:00 2001 From: Love Waern Date: Fri, 14 Jun 2024 13:53:39 +0200 Subject: [PATCH 2/2] Add a compatibility feature to suppress `WLOGMIXUP` --- RELEASENOTES.docu | 6 ++++- lib/1.2/dml-builtins.dml | 1 + lib/1.4/dml-builtins.dml | 1 + py/dml/compat.py | 24 +++++++++++++++++++ py/dml/dmlc.py | 2 ++ py/dml/messages.py | 3 +++ .../legacy/T_suppress_WBIGUNROLL_disabled.dml | 15 ++++++++++++ .../legacy/T_suppress_WBIGUNROLL_enabled.dml | 14 +++++++++++ .../suppress_WBIGUNROLL.dml} | 7 ++---- 9 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 test/1.2/legacy/T_suppress_WBIGUNROLL_disabled.dml create mode 100644 test/1.2/legacy/T_suppress_WBIGUNROLL_enabled.dml rename test/1.2/{errors/T_WBIGUNROLL.dml => legacy/suppress_WBIGUNROLL.dml} (96%) diff --git a/RELEASENOTES.docu b/RELEASENOTES.docu index 3f2b965b2..29e0cf0c7 100644 --- a/RELEASENOTES.docu +++ b/RELEASENOTES.docu @@ -1797,5 +1797,9 @@ extern typedef struct { } my_type_t; for the identifier . Added the warning WBIGUNROLL which is emitted when a #select or #foreach statement is - compiled to a large unrolled loop. + compiled to a large unrolled loop. + This warning is only enabled by default with Simics API version 8 or + above. With version 7 and below it must be explicitly enabledby passing + either --no-compat=suppress_WBIGUNROLL or + --warn=WBIGUNROLL to DMLC. diff --git a/lib/1.2/dml-builtins.dml b/lib/1.2/dml-builtins.dml index f2511de67..5a84327dc 100644 --- a/lib/1.2/dml-builtins.dml +++ b/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_suppress_WBIGUNROLL auto; parameter _compat_legacy_attributes auto; parameter _compat_lenient_typechecking auto; parameter _compat_dml12_inline auto; diff --git a/lib/1.4/dml-builtins.dml b/lib/1.4/dml-builtins.dml index fc1af1bf3..53a77309f 100644 --- a/lib/1.4/dml-builtins.dml +++ b/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_suppress_WBIGUNROLL auto; param _compat_legacy_attributes auto; param _compat_lenient_typechecking auto; param _compat_dml12_inline auto; diff --git a/py/dml/compat.py b/py/dml/compat.py index b8945d555..8a2644747 100644 --- a/py/dml/compat.py +++ b/py/dml/compat.py @@ -198,6 +198,30 @@ class suppress_WLOGMIXUP(CompatFeature): short = "Suppress the warning 'WLOGMIXUP' by default" last_api_version = api_6 +@feature +class suppress_WBIGUNROLL(CompatFeature): + '''This compatibility feature makes it so the warning `WBIGUNROLL` is + suppressed by default. `WBIGUNROLL` warns about `#foreach` and `#select` + statements that compile to large unrolled loops — for more + information, see the documentation of `WBIGUNROLL` in the + [Messages](messages.html) section. + + `WBIGUNROLL` is suppressed by default below Simics API version 8 in order + to avoid overwhelming users with warnings. Addressing occurences of large + unrolled loops should be done before or as part of migration to Simics API + version 8. + + Passing `--no-compat=suppress_WBUGUNROLL` to DMLC has almost the same effect + as passing `--warn=WBIGUNROLL`; either will cause DMLC to report the warning + even when the Simics API version in use is below 8. The only difference + between these two options is that if `--no-compat=suppress_WBIGUNROLL` is + used (and `--warn=WBIGUNROLL` is not), then `WBIGUNROLL` may still be + explicitly suppressed via `--no-warn=WBIGUNROLL`. In contrast, + `--warn=WBIGUNROLL` doesn't allow for `WBIGUNROLL` to be suppressed at + all.''' + short = "Suppress the warning 'WBIGUNROLL' by default" + last_api_version = api_7 + @feature class legacy_attributes(CompatFeature): '''This compatibility feature makes DMLC register all attributes using the diff --git a/py/dml/dmlc.py b/py/dml/dmlc.py index c1c3b39bf..8ce72098c 100644 --- a/py/dml/dmlc.py +++ b/py/dml/dmlc.py @@ -599,6 +599,8 @@ def main(argv): if compat.suppress_WLOGMIXUP in dml.globals.enabled_compat: ignore_warning('WLOGMIXUP') + if compat.suppress_WBIGUNROLL in dml.globals.enabled_compat: + ignore_warning('WBIGUNROLL') for w in options.disabled_warnings: if not is_warning_tag(w): diff --git a/py/dml/messages.py b/py/dml/messages.py index 52d9f0672..c8e69b6bf 100644 --- a/py/dml/messages.py +++ b/py/dml/messages.py @@ -2309,6 +2309,9 @@ class WBIGUNROLL(DMLWarning): list instead as a `session` array or an `extern`ed C array, and iterate through it using traditional `for` loops. For more information, see [the various answers to this Stack Overflow question.](https://stackoverflow.com/questions/75073681/cannot-use-variable-index-in-a-constant-list) + + This warning is only enabled by default with Simics API version 8 or above + (due to the compatibility feature `suppress_WBIGUNROLL`.) """ fmt = ("This '%s' statement compiles to an unrolled loop where the loop " + "body is studied and generated %s times. This may dramatically " diff --git a/test/1.2/legacy/T_suppress_WBIGUNROLL_disabled.dml b/test/1.2/legacy/T_suppress_WBIGUNROLL_disabled.dml new file mode 100644 index 000000000..5048af670 --- /dev/null +++ b/test/1.2/legacy/T_suppress_WBIGUNROLL_disabled.dml @@ -0,0 +1,15 @@ +/* + © 2024 Intel Corporation + SPDX-License-Identifier: MPL-2.0 +*/ +dml 1.2; + +device test; + +/// COMPILE-ONLY +/// NO-CC + +/// DMLC-FLAG --no-compat=suppress_WBIGUNROLL + +/// SCAN-FOR-TAGS suppress_WBIGUNROLL.dml +import "suppress_WBIGUNROLL.dml"; diff --git a/test/1.2/legacy/T_suppress_WBIGUNROLL_enabled.dml b/test/1.2/legacy/T_suppress_WBIGUNROLL_enabled.dml new file mode 100644 index 000000000..9f65ffbe9 --- /dev/null +++ b/test/1.2/legacy/T_suppress_WBIGUNROLL_enabled.dml @@ -0,0 +1,14 @@ +/* + © 2024 Intel Corporation + SPDX-License-Identifier: MPL-2.0 +*/ +dml 1.2; + +device test; + +/// COMPILE-ONLY +/// NO-CC + +/// DMLC-FLAG --simics-api=7 + +import "suppress_WBIGUNROLL.dml"; diff --git a/test/1.2/errors/T_WBIGUNROLL.dml b/test/1.2/legacy/suppress_WBIGUNROLL.dml similarity index 96% rename from test/1.2/errors/T_WBIGUNROLL.dml rename to test/1.2/legacy/suppress_WBIGUNROLL.dml index 81d63c574..acfa4aa83 100644 --- a/test/1.2/errors/T_WBIGUNROLL.dml +++ b/test/1.2/legacy/suppress_WBIGUNROLL.dml @@ -1,13 +1,10 @@ /* - © 2023 Intel Corporation + © 2024 Intel Corporation SPDX-License-Identifier: MPL-2.0 */ dml 1.2; -device test; - -/// COMPILE-ONLY -/// NO-CC +// expectations in this file are selectively enabled using SCAN-FOR-TAGS data int zero = 0;