nir/algebraic: Catch some kinds of copy-and-paste bugs in algebraic patterns
authorIan Romanick <ian.d.romanick@intel.com>
Sat, 19 Feb 2022 00:59:03 +0000 (16:59 -0800)
committerMarge Bot <emma+marge@anholt.net>
Wed, 14 Dec 2022 06:23:20 +0000 (06:23 +0000)
A later commit adds a pattern

   (('umin', ('iand', a, '#b(is_pos_power_of_two)'),
             ('iand', c, '#b(is_pos_power_of_two)')),
    ('iand', ('iand', a, b), ('iand', c, b))),

When I originally made that pattern, I copied and pasted the search to
the replacement as

  (('umin', ('iand', a, '#b(is_pos_power_of_two)'),
            ('iand', c, '#b(is_pos_power_of_two)')),
   ('iand', ('iand', a, '#b(is_pos_power_of_two)'),
            ('iand', c, '#b(is_pos_power_of_two)'))),

The caused the variables in the replacement to be marked is_constant,
and that resulted in an assertion failure deep inside nir_search.

    src/compiler/nir/nir_search.c:530: construct_value: Assertion `!var->is_constant' failed.

These extra validation rules catch this kind of error at compile time
rather than at run time.

Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Reviewed-by: Jason Ekstrand <jason.ekstrand@collabora.com>
Acked-by: Jesse Natalie <jenatali@microsoft.com>
Tested-by: Daniel Schürmann <daniel@schuermann.dev>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15121>

src/compiler/nir/nir_algebraic.py

index 2adc4d0..4a2e438 100644 (file)
@@ -739,6 +739,17 @@ class BitSizeValidator(object):
       if isinstance(val, Expression):
          for src in val.sources:
             self.validate_replace(src, search)
+      elif isinstance(val, Variable):
+          # These catch problems when someone copies and pastes the search
+          # into the replacement.
+          assert not val.is_constant, \
+              'Replacement variables must not be marked constant.'
+
+          assert val.cond_index == -1, \
+              'Replacement variables must not have a condition.'
+
+          assert not val.required_type, \
+              'Replacement variables must not have a required type.'
 
    def validate(self, search, replace):
       self.is_search = True