From a834aeef0fa2f1c1182b1e61696af2c22380e28e Mon Sep 17 00:00:00 2001 From: Matt Wala Date: Fri, 23 Dec 2016 12:40:15 -0600 Subject: [PATCH 1/6] Implement a syntax for no_sync_with_scopes. (closes #67 on github) --- loopy/kernel/__init__.py | 5 ++--- loopy/kernel/creation.py | 38 ++++++++++++++++++++++++++++--------- loopy/kernel/instruction.py | 4 +--- test/test_loopy.py | 19 +++++++++++++++++++ 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/loopy/kernel/__init__.py b/loopy/kernel/__init__.py index 5dff5e53c..e48b83935 100644 --- a/loopy/kernel/__init__.py +++ b/loopy/kernel/__init__.py @@ -1226,9 +1226,8 @@ class LoopKernel(ImmutableRecordWithoutPickling): options.append( "conflicts=%s" % ":".join(insn.conflicts_with_groups)) if insn.no_sync_with: - # FIXME: Find a syntax to express scopes. - options.append("no_sync_with=%s" % ":".join(id for id, _ in - insn.no_sync_with)) + options.append("no_sync_with=%s" % ":".join( + "%s@%s" % entry for entry in sorted(insn.no_sync_with))) if lhs: core = "%s <- %s" % ( diff --git a/loopy/kernel/creation.py b/loopy/kernel/creation.py index 0a030b108..9a05408bb 100644 --- a/loopy/kernel/creation.py +++ b/loopy/kernel/creation.py @@ -167,6 +167,11 @@ def get_default_insn_options_dict(): } +from collections import namedtuple + +_NosyncParseResult = namedtuple("_NosyncParseResult", "expr, scope") + + def parse_insn_options(opt_dict, options_str, assignee_names=None): if options_str is None: return opt_dict @@ -175,6 +180,20 @@ def parse_insn_options(opt_dict, options_str, assignee_names=None): result = opt_dict.copy() + def parse_nosync_option(opt_value): + if "@" in opt_value: + expr, scope = opt_value.split("@") + else: + expr = opt_value + scope = "any" + allowable_scopes = ("local", "global", "any") + if scope not in allowable_scopes: + raise ValueError( + "unknown scope for nosync option: '%s' " + "(allowable scopes are %s)" % + (scope, ', '.join("'%s'" % s for s in allowable_scopes))) + return _NosyncParseResult(expr, scope) + for option in options_str.split(","): option = option.strip() if not option: @@ -242,23 +261,24 @@ def parse_insn_options(opt_dict, options_str, assignee_names=None): raise LoopyError("'nosync' option may not be specified " "in a 'with' block") - # TODO: Come up with a syntax that allows the user to express - # different synchronization scopes. result["no_sync_with"] = result["no_sync_with"].union(frozenset( - (intern(dep.strip()), "any") - for dep in opt_value.split(":") if dep.strip())) + (option.expr.strip(), option.scope) + for option in ( + parse_nosync_option(entry) + for entry in opt_value.split(":")) + if option.expr.strip())) elif opt_key == "nosync_query" and opt_value is not None: if is_with_block: raise LoopyError("'nosync' option may not be specified " "in a 'with' block") + match_expr, scope = parse_nosync_option(opt_value) + from loopy.match import parse_match - match = parse_match(opt_value) - # TODO: Come up with a syntax that allows the user to express - # different synchronization scopes. + match = parse_match(match_expr) result["no_sync_with"] = result["no_sync_with"].union( - frozenset([(match, "any")])) + frozenset([(match, scope)])) elif opt_key == "groups" and opt_value is not None: result["groups"] = frozenset( @@ -1627,7 +1647,7 @@ def resolve_dependencies(knl): (resolved_insn_id, nosync_scope) for nosync_dep, nosync_scope in insn.no_sync_with for resolved_insn_id in - _resolve_dependencies(knl, insn, nosync_dep)), + _resolve_dependencies(knl, insn, (nosync_dep,))), )) return knl.copy(instructions=new_insns) diff --git a/loopy/kernel/instruction.py b/loopy/kernel/instruction.py index 4477f5baf..2e81c2e38 100644 --- a/loopy/kernel/instruction.py +++ b/loopy/kernel/instruction.py @@ -388,10 +388,8 @@ class InstructionBase(ImmutableRecord): if self.depends_on: result.append("dep="+":".join(self.depends_on)) if self.no_sync_with: - # TODO: Come up with a syntax to express different kinds of - # synchronization scopes. result.append("nosync="+":".join( - insn_id for insn_id, _ in self.no_sync_with)) + "%s@%s" % entry for entry in self.no_sync_with)) if self.groups: result.append("groups=%s" % ":".join(self.groups)) if self.conflicts_with_groups: diff --git a/test/test_loopy.py b/test/test_loopy.py index 48ccd8ee0..6ef28f676 100644 --- a/test/test_loopy.py +++ b/test/test_loopy.py @@ -1995,6 +1995,25 @@ def test_integer_reduction(ctx_factory): assert function(out) +def test_nosync_option_parsing(): + knl = lp.make_kernel( + "{[i]: 0 <= i < 10}", + """ + <>t = 1 {id=insn1,nosync=insn1} + t = 2 {id=insn2,nosync=insn1:insn2} + t = 3 {id=insn3,nosync=insn1@local:insn2@global:insn3@any} + t = 4 {id=insn4,nosync_query=id:insn*@local} + t = 5 {id=insn5,nosync_query=id:insn1} + """, + options=lp.Options(allow_terminal_colors=False)) + kernel_str = str(knl) + assert "# insn1,no_sync_with=insn1@any" in kernel_str + assert "# insn2,no_sync_with=insn1@any:insn2@any" in kernel_str + assert "# insn3,no_sync_with=insn1@local:insn2@global:insn3@any" in kernel_str + assert "# insn4,no_sync_with=insn1@local:insn2@local:insn3@local" in kernel_str + assert "# insn5,no_sync_with=insn1@any" in kernel_str + + def assert_barrier_between(knl, id1, id2): from loopy.schedule import RunInstruction, Barrier watch_for_barrier = False -- GitLab From 832744078f0a8a7d873a67a9c2b25651724ca2a8 Mon Sep 17 00:00:00 2001 From: Matt Wala Date: Sun, 22 Jan 2017 17:08:50 -0600 Subject: [PATCH 2/6] Add missing substring to test result. --- test/test_loopy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_loopy.py b/test/test_loopy.py index 05d2544af..ba20b5866 100644 --- a/test/test_loopy.py +++ b/test/test_loopy.py @@ -2010,7 +2010,7 @@ def test_nosync_option_parsing(): assert "# insn1,no_sync_with=insn1@any" in kernel_str assert "# insn2,no_sync_with=insn1@any:insn2@any" in kernel_str assert "# insn3,no_sync_with=insn1@local:insn2@global:insn3@any" in kernel_str - assert "# insn4,no_sync_with=insn1@local:insn2@local:insn3@local" in kernel_str + assert "# insn4,no_sync_with=insn1@local:insn2@local:insn3@local:insn5@local" in kernel_str # noqa assert "# insn5,no_sync_with=insn1@any" in kernel_str -- GitLab From 12a10b0b725d35240e482c9ae6fbcbd07d460fa2 Mon Sep 17 00:00:00 2001 From: Matt Wala Date: Sun, 22 Jan 2017 17:31:46 -0600 Subject: [PATCH 3/6] Document nosync changes. --- doc/ref_kernel.rst | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/doc/ref_kernel.rst b/doc/ref_kernel.rst index 2d754dec2..19a4e0e4d 100644 --- a/doc/ref_kernel.rst +++ b/doc/ref_kernel.rst @@ -248,8 +248,8 @@ These are usually key-value pairs. The following attributes are recognized: expression is used to match instructions in the kernel and add them as dependencies. -* ``nosync=id1:id2`` prescribes that no barrier synchronization is necessary - the instructions with identifiers ``id1`` and ``id2`` to the, even if +* ``nosync=id1:id2`` prescribes that no barrier synchronization is + necessary the instructions with identifiers ``id1`` and ``id2``, even if a dependency chain exists and variables are accessed in an apparently racy way. @@ -257,8 +257,24 @@ These are usually key-value pairs. The following attributes are recognized: function :func:`fnmatch.fnmatchcase`. This is helpful in conjunction with ``id_prefix``. + Identifiers (including wildcards) accept an optional `@scope` suffix, + which prescribes that no synchronization at level *scope* is needed. + This does not preclude barriers at levels different from *scope*. + Allowable scope values are: + + * `local` + * `global` + * `any` + + As an example, ``nosync=id1@local:id2@global`` prescribes that no local + synchronization is needed with instruction ``id1`` and no global + synchronization is needed with instruction ``id2``. + + ``nosync=id1@any`` has the same effect as ``nosync=id1``. + * ``nosync_query=...`` provides an alternative way of specifying ``nosync``, - just like ``dep_query`` and ``dep``. + just like ``dep_query`` and ``dep``. As with ``nosync``, ``nosync_query`` + accepts an optional `@scope` suffix. * ``priority=integer`` sets the instructions priority to the value ``integer``. Instructions with higher priority will be scheduled sooner, -- GitLab From ab6ba209eccb7836cb60b219f7457c71124f1764 Mon Sep 17 00:00:00 2001 From: Matt Wala Date: Sun, 22 Jan 2017 17:35:42 -0600 Subject: [PATCH 4/6] Small grammar tweaks. --- doc/ref_kernel.rst | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/ref_kernel.rst b/doc/ref_kernel.rst index 19a4e0e4d..867443408 100644 --- a/doc/ref_kernel.rst +++ b/doc/ref_kernel.rst @@ -248,19 +248,19 @@ These are usually key-value pairs. The following attributes are recognized: expression is used to match instructions in the kernel and add them as dependencies. -* ``nosync=id1:id2`` prescribes that no barrier synchronization is - necessary the instructions with identifiers ``id1`` and ``id2``, even if - a dependency chain exists and variables are accessed in an apparently - racy way. +* ``nosync=id1:id2`` prescribes that no barrier synchronization is necessary + for the instructions with identifiers ``id1`` and ``id2``, even if a + dependency chain exists and variables are accessed in an apparently racy + way. Identifiers here are allowed to be wildcards as defined by the Python function :func:`fnmatch.fnmatchcase`. This is helpful in conjunction with ``id_prefix``. Identifiers (including wildcards) accept an optional `@scope` suffix, - which prescribes that no synchronization at level *scope* is needed. - This does not preclude barriers at levels different from *scope*. - Allowable scope values are: + which prescribes that no synchronization at level `scope` is needed. + This does not preclude barriers at levels different from `scope`. + Allowable `scope` values are: * `local` * `global` -- GitLab From d51b26167417e75165ef113060df007b4e2dc2e7 Mon Sep 17 00:00:00 2001 From: Matt Wala Date: Mon, 23 Jan 2017 10:16:33 -0600 Subject: [PATCH 5/6] for -> due to --- doc/ref_kernel.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ref_kernel.rst b/doc/ref_kernel.rst index 867443408..e6447a6f5 100644 --- a/doc/ref_kernel.rst +++ b/doc/ref_kernel.rst @@ -249,7 +249,7 @@ These are usually key-value pairs. The following attributes are recognized: dependencies. * ``nosync=id1:id2`` prescribes that no barrier synchronization is necessary - for the instructions with identifiers ``id1`` and ``id2``, even if a + due to the instructions with identifiers ``id1`` and ``id2``, even if a dependency chain exists and variables are accessed in an apparently racy way. -- GitLab From 5c9715c502a79f8b0d33b95bea96887d6a9d441f Mon Sep 17 00:00:00 2001 From: Matt Wala Date: Mon, 23 Jan 2017 10:18:25 -0600 Subject: [PATCH 6/6] Revert "for -> due to" (I changed my mind) This reverts commit d51b26167417e75165ef113060df007b4e2dc2e7. --- doc/ref_kernel.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ref_kernel.rst b/doc/ref_kernel.rst index e6447a6f5..867443408 100644 --- a/doc/ref_kernel.rst +++ b/doc/ref_kernel.rst @@ -249,7 +249,7 @@ These are usually key-value pairs. The following attributes are recognized: dependencies. * ``nosync=id1:id2`` prescribes that no barrier synchronization is necessary - due to the instructions with identifiers ``id1`` and ``id2``, even if a + for the instructions with identifiers ``id1`` and ``id2``, even if a dependency chain exists and variables are accessed in an apparently racy way. -- GitLab