Merge pull request #245 from inducer/pytools-tag-for-instruction

No more string tags

Author: inducer

doc/ref_kernel.rst

@@ -493,6 +493,12 @@ Barrier Instructions
 
 .. autoclass:: BarrierInstruction
 
+Instruction Tags
+^^^^^^^^^^^^^^^^
+
+.. autoclass:: LegacyStringInstructionTag
+.. autoclass:: UseStreamingStoreTag
+
 .. }}}
 
 Data: Arguments and Temporaries

loopy/__init__.py

@@ -32,6 +32,7 @@
         default_function_mangler, single_arg_function_mangler)
 
 from loopy.kernel.instruction import (
+        LegacyStringInstructionTag, UseStreamingStoreTag,
         MemoryOrdering, memory_ordering,
         MemoryScope, memory_scope,
         VarAtomicity, OrderedAtomic, AtomicInit, AtomicUpdate,
@@ -155,6 +156,7 @@
         "LoopKernel",
         "KernelState", "kernel_state",  # lower case is deprecated
 
+        "LegacyStringInstructionTag", "UseStreamingStoreTag",
         "MemoryOrdering", "memory_ordering",  # lower case is deprecated
         "MemoryScope", "memory_scope",  # lower case is deprecated
 

loopy/frontend/fortran/translator.py

@@ -34,6 +34,7 @@
 from islpy import dim_type
 from loopy.symbolic import IdentityMapper
 from loopy.diagnostic import LoopyError
+from loopy.kernel.instruction import LegacyStringInstructionTag
 from pymbolic.primitives import Wildcard
 
 
@@ -640,16 +641,16 @@ def map_Comment(self, node):
                 stripped_comment_line)
 
         if begin_tag_match:
-            tag = begin_tag_match.group(1)
+            tag = LegacyStringInstructionTag(begin_tag_match.group(1))
             if tag in self.instruction_tags:
-                raise TranslationError("nested begin tag for tag '%s'" % tag)
+                raise TranslationError(f"nested begin tag for tag '{tag.value}'")
             self.instruction_tags.append(tag)
 
         elif end_tag_match:
-            tag = end_tag_match.group(1)
+            tag = LegacyStringInstructionTag(end_tag_match.group(1))
             if tag not in self.instruction_tags:
                 raise TranslationError(
-                        "end tag without begin tag for tag '%s'" % tag)
+                        f"end tag without begin tag for tag '{tag.value}'")
             self.instruction_tags.remove(tag)
 
         elif faulty_loopy_pragma_match is not None:

loopy/kernel/creation.py

@@ -72,6 +72,38 @@ def __init__(self, name):
 # }}}
 
 
+# {{{ tag normalization
+
+def _normalize_string_tag(tag):
+    from pytools.tag import Tag
+
+    from loopy.kernel.instruction import (
+            UseStreamingStoreTag, LegacyStringInstructionTag)
+    if tag == "!streaming_store":
+        return UseStreamingStoreTag()
+    else:
+        from pytools import resolve_name
+        try:
+            tag_cls = resolve_name(tag)
+        except ImportError:
+            pass
+        except AttributeError:
+            pass
+        else:
+            if issubclass(tag_cls, Tag):
+                return tag_cls()
+
+        return LegacyStringInstructionTag(tag)
+
+
+def _normalize_tags(tags):
+    return frozenset(
+                    _normalize_string_tag(t) if isinstance(t, str) else t
+                    for t in tags)
+
+# }}}
+
+
 # {{{ expand defines
 
 WORD_RE = re.compile(r"\b([a-zA-Z0-9_]+)\b")
@@ -328,9 +360,9 @@ def parse_nosync_option(opt_value):
             del new_predicates
 
         elif opt_key == "tags" and opt_value is not None:
-            result["tags"] = frozenset(
+            result["tags"] = _normalize_tags([
                     tag.strip() for tag in opt_value.split(":")
-                    if tag.strip())
+                    if tag.strip()])
 
         elif opt_key == "atomic":
             if is_with_block:
@@ -803,6 +835,10 @@ def intern_if_str(s):
                             | insn_options_stack[-1]["conflicts_with_groups"]),
                         **kwargs)
 
+            norm_tags = _normalize_tags(insn.tags)
+            if norm_tags != insn.tags:
+                insn = insn.copy(tags=norm_tags)
+
             new_instructions.append(insn)
             inames_to_dup.append([])
 

loopy/kernel/instruction.py

@@ -22,15 +22,59 @@
 
 from sys import intern
 from pytools import ImmutableRecord, memoize_method
+from pytools.tag import Tag, tag_dataclass, Taggable
 from loopy.diagnostic import LoopyError
 from loopy.tools import Optional
 from warnings import warn
 import islpy as isl
 
 
+# {{{ instruction tags
+
+@tag_dataclass
+class LegacyStringInstructionTag(Tag):
+    """A subclass of :class:`pytools.tag.Tag` for use in
+    :attr:`InstructionBase.tags` used for forward compatibility of the old
+    string-based tagging mechanism. String-based tags are automatically converted
+    to this type.
+
+    .. attribute:: value
+    """
+    value: str
+
+    # FIXME: This class should be deprecated as soon as there is a viable
+    # alternative. For now, pattern matching and the textual syntax are
+    # only able to generate string tags, which is why the deprecation is not
+    # yet in effect.
+
+
+@tag_dataclass
+class UseStreamingStoreTag(Tag):
+    """A subclass of :class:`pytools.tag.Tag` for use in
+    :attr:`InstructionBase.tags` used to indicate that if the instruction is an
+    :class:`Assignment` or a :class:`CallInstruction`, then the 'store' part of
+    the assignment should be realized using streaming stores.
+
+    .. note::
+
+        This tag is advisory in nature and may be ignored by targets
+        that do not understand it or in situations where it does not
+        apply.
+
+    .. warning::
+
+        This is a dodgy shortcut, and no promise is made that this will
+        continue to work. Whether this is safe is target-dependent and
+        program-dependent. No promise of safety is made.
+    """
+    pass
+
+# }}}
+
+
 # {{{ instructions: base class
 
-class InstructionBase(ImmutableRecord):
+class InstructionBase(ImmutableRecord, Taggable):
     """A base class for all types of instruction that can occur in
     a kernel.
 
@@ -135,11 +179,11 @@ class InstructionBase(ImmutableRecord):
 
     .. attribute:: tags
 
-        A :class:`frozenset` of string identifiers that can be used to
-        identify groups of instructions.
-
-        Tags starting with exclamation marks (``!``) are reserved and may have
-        specific meanings defined by :mod:`loopy` or its targets.
+        A :class:`frozenset` of subclasses of :class:`pytools.tag.Tag` used to
+        provide metadata on this object. Legacy string tags are converted to
+        :class:`LegacyStringInstructionTag` or, if they used to carry
+        a functional meaning, the tag carrying that same fucntional meaning
+        (e.g. :class:`UseStreamingStoreTag`).
 
     .. automethod:: __init__
     .. automethod:: assignee_var_names
@@ -148,6 +192,8 @@ class InstructionBase(ImmutableRecord):
     .. automethod:: write_dependency_names
     .. automethod:: dependency_names
     .. automethod:: copy
+
+    Inherits from :class:`pytools.tag.Taggable`.
     """
 
     # within_inames_is_final is deprecated and will be removed in version 2017.x.
@@ -257,8 +303,13 @@ def __init__(self, id, depends_on, depends_on_is_final,
                 within_inames=within_inames,
                 priority=priority,
                 predicates=predicates,
+                # Yes, tags is set by both this and the Taggable constructor.
+                # Here, we set it so that ImmutableRecord knows about it.
+                # The Taggable constructor call does extra validation.
                 tags=tags)
 
+        Taggable.__init__(self, tags)
+
     # {{{ abstract interface
 
     def read_dependency_names(self):
@@ -347,7 +398,9 @@ def get_str_options(self):
         if self.priority:
             result.append("priority=%d" % self.priority)
         if self.tags:
-            result.append("tags=%s" % ":".join(self.tags))
+            from loopy.kernel.tools import stringify_instruction_tag
+            result.append("tags=%s" % ":".join(
+                stringify_instruction_tag(t) for t in self.tags))
         if hasattr(self, "atomicity") and self.atomicity:
             result.append("atomic=%s" % ":".join(str(a) for a in self.atomicity))
 

loopy/kernel/tools.py

@@ -1415,6 +1415,14 @@ def conform_to_uniform_length(s):
 
 # {{{ stringify_instruction_list
 
+def stringify_instruction_tag(tag):
+    from loopy.kernel.instruction import LegacyStringInstructionTag
+    if isinstance(tag, LegacyStringInstructionTag):
+        return f"S({tag.value})"
+    else:
+        return str(tag)
+
+
 def stringify_instruction_list(kernel):
     # {{{ topological sort
 
@@ -1529,7 +1537,8 @@ def adapt_to_new_inames_list(new_inames):
         if insn.priority:
             options.append("priority=%d" % insn.priority)
         if insn.tags:
-            options.append("tags=%s" % ":".join(insn.tags))
+            options.append("tags=%s" % ":".join(
+                stringify_instruction_tag(t) for t in insn.tags))
         if isinstance(insn, lp.Assignment) and insn.atomicity:
             options.append("atomic=%s" % ":".join(
                 str(a) for a in insn.atomicity))

loopy/match.py

@@ -243,8 +243,16 @@ def __call__(self, kernel, matchable):
 
 class Tagged(GlobMatchExpressionBase):
     def __call__(self, kernel, matchable):
+        from loopy.kernel.instruction import LegacyStringInstructionTag
         if matchable.tags:
-            return any(self.re.match(tag) for tag in matchable.tags)
+            return any(
+                    self.re.match(tag.value)
+                    if isinstance(tag, LegacyStringInstructionTag)
+                    else
+
+                    False
+
+                    for tag in matchable.tags)
         else:
             return False
 

loopy/statistics.py

@@ -589,10 +589,11 @@ class MemAccess(Record):
        A :class:`str` that specifies the variable name of the data
        accessed.
 
-    .. attribute:: variable_tag
+    .. attribute:: variable_tags
 
-       A :class:`str` that specifies the variable tag of a
-       :class:`loopy.symbolic.TaggedVariable`.
+       A :class:`frozenset` of subclasses of :class:`~pytools.tag.Tag`
+       that reflects :attr:`~loopy.symbolic.TaggedVariable.tags` of
+       an accessed variable.
 
     .. attribute:: count_granularity
 
@@ -610,27 +611,60 @@ class MemAccess(Record):
     """
 
     def __init__(self, mtype=None, dtype=None, lid_strides=None, gid_strides=None,
-                 direction=None, variable=None, variable_tag=None,
+                 direction=None, variable=None,
+                 *, variable_tags=None, variable_tag=None,
                  count_granularity=None):
 
         if count_granularity not in CountGranularity.ALL+[None]:
             raise ValueError("Op.__init__: count_granularity '%s' is "
                     "not allowed. count_granularity options: %s"
                     % (count_granularity, CountGranularity.ALL+[None]))
 
+        # {{{ normalize variable_tags
+
+        if variable_tags is not None and variable_tag is not None:
+            raise TypeError(
+                    "may not specify both 'variable_tags' and 'variable_tag'")
+        if variable_tag is not None:
+            from loopy.kernel.creation import _normalize_string_tag
+            variable_tags = frozenset({_normalize_string_tag(variable_tag)})
+
+            from warnings import warn
+            warn("Passing 'variable_tag' to MemAccess is deprecated and will "
+                    "stop working in 2022. Pass variable_tags instead.")
+
+        if variable_tags is None:
+            variable_tags = frozenset()
+
+        # }}}
+
         if dtype is None:
             Record.__init__(self, mtype=mtype, dtype=dtype, lid_strides=lid_strides,
                             gid_strides=gid_strides, direction=direction,
-                            variable=variable, variable_tag=variable_tag,
+                            variable=variable, variable_tags=variable_tags,
                             count_granularity=count_granularity)
         else:
             from loopy.types import to_loopy_type
             Record.__init__(self, mtype=mtype, dtype=to_loopy_type(dtype),
                             lid_strides=lid_strides, gid_strides=gid_strides,
                             direction=direction, variable=variable,
-                            variable_tag=variable_tag,
+                            variable_tags=variable_tags,
                             count_granularity=count_granularity)
 
+    @property
+    def variable_tag(self):
+        from warnings import warn
+        warn("Accessing MemAccess.variable_tag is deprecated and will stop working "
+                "in 2022. Use MemAccess.variable_tags instead.", DeprecationWarning,
+                stacklevel=2)
+
+        if len(self.variable_tags) != 1:
+            raise ValueError("cannot access MemAccess.variable_tag: access has "
+                    f"{len(self.variable_tags)} tags")
+
+        tag, = self.variable_tags
+        return tag
+
     def __hash__(self):
         # Note that this means lid_strides and gid_strides must be sorted
         # in self.__repr__()
@@ -647,7 +681,7 @@ def __repr__(self):
                 sorted(self.gid_strides.items())),
             self.direction,
             self.variable,
-            self.variable_tag,
+            self.variable_tags,
             self.count_granularity)
 
 # }}}
@@ -1031,9 +1065,9 @@ def map_variable(self, expr):
     def map_subscript(self, expr):
         name = expr.aggregate.name
         try:
-            var_tag = expr.aggregate.tag
+            var_tags = expr.aggregate.tags
         except AttributeError:
-            var_tag = None
+            var_tags = frozenset()
 
         if name in self.knl.arg_dict:
             array = self.knl.arg_dict[name]
@@ -1062,7 +1096,7 @@ def map_subscript(self, expr):
                             lid_strides=dict(sorted(lid_strides.items())),
                             gid_strides=dict(sorted(gid_strides.items())),
                             variable=name,
-                            variable_tag=var_tag,
+                            variable_tags=var_tags,
                             count_granularity=count_granularity
                             ): 1}
                           ) + self.rec(expr.index_tuple)
@@ -1678,7 +1712,7 @@ def get_mem_access_map(knl, numpy_types=True, count_redundant_work=False,
                             gid_strides=mem_access.gid_strides,
                             direction=mem_access.direction,
                             variable=mem_access.variable,
-                            variable_tag=mem_access.variable_tag,
+                            variable_tags=mem_access.variable_tags,
                             count_granularity=mem_access.count_granularity):
                         ct
                         for mem_access, ct in access_map.count_map.items()},