[Bug tree-optimization/108129] New: nop_atomic_bit_test_and_p is too bloated

[Bug tree-optimization/108129] New: nop_atomic_bit_test_and_p is too bloated

[amonakov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108129

            Bug ID: 108129
           Summary: nop_atomic_bit_test_and_p is too bloated
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: amonakov at gcc dot gnu.org
  Target Milestone: ---

match.pd has multi-pattern matcher 'nop_atomic_bit_test_and_p'.

It expands to ~38 KLOC in gimple-match.cc and ~350 KB in the compiled binary.

There has to be a better way than repeatedly emitting the match pattern for
each member of {ATOMIC,SYNC}_FETCH_{AND,OR_XOR}_N :)

[Bug tree-optimization/108129] nop_atomic_bit_test_and_p is too bloated

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108129

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-12-15
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Yep, I also noticed that some time ago :)

[Bug tree-optimization/108129] nop_atomic_bit_test_and_p is too bloated

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108129

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This might improve the build time of GCC ...

[Bug tree-optimization/108129] nop_atomic_bit_test_and_p is too bloated

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108129

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Alexander Monakov from comment #0)
> match.pd has multi-pattern matcher 'nop_atomic_bit_test_and_p'.
> 
> It expands to ~38 KLOC in gimple-match.cc and ~350 KB in the compiled binary.
> 
> There has to be a better way than repeatedly emitting the match pattern for
> each member of {ATOMIC,SYNC}_FETCH_{AND,OR_XOR}_N :)

It's the way the matcher works - if you can think of a better way of
code-generating it within the constraints:
 - earlier patterns in match.pd should match first in case there are multiple
matches
 - matching time should ideally be O(size of the pattern) rather than O(size of
match.pd)

The 2nd constraint isn't currently fulfilled because of the first constraint
(or how that is implemented).  The 2nd constraint was the original design
goal to make frequent "re-folding" cheap.  The first constraint was implemented
to allow pattern ordering to disambiguate "overlapping" matches.

Writing a different code generation in genmatch should be possible, the
(lowered) AST is available.

[Bug tree-optimization/108129] nop_atomic_bit_test_and_p is too bloated

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108129

--- Comment #4 from Hongtao.liu <crazylht at gmail dot com> ---
We may merge ATOMIC_XXX_N with SYNC_XXX_N if the "?" can be extent to optional
operand.

The only difference between them is ATOMIC_XXX_N has one more parameter, and
it's not used by the pattern match.

Currently "?" only support optional unary operation.
***
The support for ? marking extends to all unary operations including predicates
you
declare yourself with match.***

[Bug tree-optimization/108129] nop_atomic_bit_test_and_p is too bloated

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108129

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
I have the matching code down to ~4k lines of code.

[Bug tree-optimization/108129] nop_atomic_bit_test_and_p is too bloated

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108129

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:75cda3be0232f745cda4e177d514f6900390af0b

commit r13-6902-g75cda3be0232f745cda4e177d514f6900390af0b
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Mar 28 12:42:14 2023 +0200

    bootstrap/84402 - improve (match ...) code generation

    The following avoids duplicating matching code for (match ...)
    in match.pd when possible.  That's more easily possible for
    (match ...) than simplify because we do not need to handle
    common matches (those would be diagnosed only during compiling)
    nor is the result able to inspect the active operator.

    Specifically this reduces the size of the generated matches for
    the atomic ops as noted in PR108129.

    gimple-match.cc shrinks from 245k lines to 209k lines with this patch.

            PR bootstrap/84402
            PR tree-optimization/108129
            * genmatch.cc (lower_for): For (match ...) delay
            substituting into the match operator if possible.
            (dt_operand::gen_gimple_expr): For user_id look at the
            first substitute for determining how to access operands.
            (dt_operand::gen_generic_expr): Likewise.
            (dt_node::gen_kids): Properly sort user_ids according
            to their substitutes.
            (dt_node::gen_kids_1): Code-generate user_id matching.

[Bug tree-optimization/108129] nop_atomic_bit_test_and_p is too bloated

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108129

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think we can consider this one fixed.  (it's still 2% of gimple-match.cc)