Bug 108129 - nop_atomic_bit_test_and_p is too bloated
Summary: nop_atomic_bit_test_and_p is too bloated
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: ---
Assignee: Richard Biener
URL:
Keywords: internal-improvement
Depends on:
Blocks:
 
Reported: 2022-12-15 15:06 UTC by Alexander Monakov
Modified: 2023-03-28 11:35 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-12-15 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Monakov 2022-12-15 15:06:57 UTC
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 :)
Comment 1 Martin Liška 2022-12-15 15:11:48 UTC
Yep, I also noticed that some time ago :)
Comment 2 Andrew Pinski 2022-12-15 17:24:55 UTC
This might improve the build time of GCC ...
Comment 3 Richard Biener 2022-12-15 17:57:35 UTC
(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.
Comment 4 Hongtao.liu 2023-03-28 04:34:30 UTC
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.***
Comment 5 Richard Biener 2023-03-28 10:51:10 UTC
I have the matching code down to ~4k lines of code.
Comment 6 GCC Commits 2023-03-28 11:31:20 UTC
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.
Comment 7 Richard Biener 2023-03-28 11:35:46 UTC
I think we can consider this one fixed.  (it's still 2% of gimple-match.cc)