Re: [PATCH 1/2] gimplefe: Add support for REF_REVERSE_STORAGE_ORDER on mem
Richard Biener <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
> Am 10.08.2026 um 17:45 schrieb Richard Biener <[email protected]>: > > > >>> Am 10.08.2026 um 17:39 schrieb Andrea Pinski <[email protected]>: >>> >>> On Mon, Aug 10, 2026 at 1:58 AM Richard Biener >>> <[email protected]> wrote: >>> >>>> On Sun, Aug 9, 2026 at 12:20 AM Andrea Pinski >>>> <[email protected]> wrote: >>>> >>>> While fixing PR 126729 and PR 126570, I found it was hard >>>> to figure out if REF_REVERSE_STORAGE_ORDER was set on a MEM_REF. >>>> Even for the gimple fe was able to set it. >>>> This adds support to the gimple fe and also outputs >>>> REF_REVERSE_STORAGE_ORDER for MEM_REF in gimple mode. >>>> >>>> Bootstrapped and tested on x84_64-linux-gnu. >>>> >>>> gcc/c/ChangeLog: >>>> >>>> * gimple-parser.cc (c_parser_gimple_postfix_expression): >>>> Allow for an optional `, 1/0` in front of the cb pair >>>> for MEM_REF. >>>> >>>> gcc/ChangeLog: >>>> >>>> * tree-pretty-print.cc (dump_mem_ref): Dump REF_REVERSE_STORAGE_ORDER >>>> if it was set. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> * gcc.dg/gimplefe-59.c: New test. >>>> >>>> Signed-off-by: Andrea Pinski <[email protected]> >>>> --- >>>> gcc/c/gimple-parser.cc | 34 ++++++++++++++++++++++++++++-- >>>> gcc/testsuite/gcc.dg/gimplefe-59.c | 16 ++++++++++++++ >>>> gcc/tree-pretty-print.cc | 6 ++++++ >>>> 3 files changed, 54 insertions(+), 2 deletions(-) >>>> create mode 100644 gcc/testsuite/gcc.dg/gimplefe-59.c >>>> >>>> diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc >>>> index 0fcb9ead83d..59cb29f420c 100644 >>>> --- a/gcc/c/gimple-parser.cc >>>> +++ b/gcc/c/gimple-parser.cc >>>> @@ -1530,6 +1530,7 @@ c_parser_gimple_postfix_expression (gimple_parser &parser) >>>> /* __MEM '<' type-name [ ',' number ] '>' >>>> '(' [ '(' type-name ')' ] unary-expression >>>> [ '+' number ] >>>> + [ ',' number ] >>> >>> But this breaks existing IL, no? Also, why a number ... and why in >>> the deref part? >> >> No it does not break existing IL because the next part contains a `:` >> in its argument so it is easy to check. >> The reason for the number is it is easier to parse than anything else > > But a -GIMPLE dump should be as easy to parse than a regular one (did you adjust dumping)? I don’t like a 0/1 here. > >> >>> I'd have expected this to be part of the <type-name ...> part, >>> alongside the alignment >>> specification? >> >> Alignment is part of the type while reverse is part of the mem_ref. >> I know it is a bit odd but that is the current situation we are in. > > Note that we do not have to match 1:1, but I see your point. IIRC it can also be on a CONPONENT_REF >> >>> >>>> [ ',' number ':' number ] ')' */ >>>> location_t loc = c_parser_peek_token (parser)->location; >>>> c_parser_consume_token (parser); >>>> @@ -1542,6 +1543,8 @@ c_parser_gimple_postfix_expression (gimple_parser &parser) >>>> index2.value = NULL_TREE; >>>> unsigned short clique = 0; >>>> unsigned short base = 0; >>>> + bool reverse_order = false; >>>> + struct c_expr ro; >>>> if (c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) >>>> { >>>> tree alias_type = NULL_TREE; >>>> @@ -1626,10 +1629,29 @@ c_parser_gimple_postfix_expression (gimple_parser &parser) >>>> if (c_parser_next_token_is (parser, CPP_COMMA)) >>>> { >>>> struct c_expr cl, ba; >>>> + bool has_clb = true; >>>> c_parser_consume_token (parser); >>>> cl = c_parser_gimple_postfix_expression (parser); >>>> - if (c_parser_require (parser, >>>> - CPP_COLON, "expected %<:%>")) >>>> + if (!c_parser_next_token_is (parser, CPP_COLON)) >>>> + { >>>> + ro = cl; >>>> + unsigned HOST_WIDE_INT tmp = 0; >>>> + if (!tree_fits_uhwi_p (ro.value) >>>> + || (tmp = tree_to_uhwi (ro.value)) > 1) >>>> + error_at (ro.get_start (), >>>> + "invalid reverse order value"); >>>> + reverse_order = tmp; >>>> + has_clb = false; >>>> + if (c_parser_next_token_is (parser, CPP_COMMA)) >>>> + { >>>> + c_parser_consume_token (parser); >>>> + cl = c_parser_gimple_postfix_expression (parser); >>>> + has_clb = true; >>>> + } >>>> + } >>>> + if (has_clb >>>> + && c_parser_require (parser, >>>> + CPP_COLON, "expected %<:%>")) >>>> { >>>> ba = c_parser_gimple_postfix_expression (parser); >>>> if (!tree_fits_uhwi_p (cl.value) >>>> @@ -1665,6 +1687,14 @@ c_parser_gimple_postfix_expression (gimple_parser &parser) >>>> MR_DEPENDENCE_CLIQUE (expr.value) = clique; >>>> MR_DEPENDENCE_BASE (expr.value) = base; >>>> } >>>> + if (reverse_order) >>>> + { >>>> + if (TREE_CODE (expr.value) == MEM_REF) >>>> + REF_REVERSE_STORAGE_ORDER (expr.value) = reverse_order; >>>> + else >>>> + error_at (ro.get_start (), >>>> + "target mem ref cannot have reverse order"); >>>> + } >>>> break; >>>> } >>>> else if (strcmp (IDENTIFIER_POINTER (id), "__VIEW_CONVERT") == 0) >>>> diff --git a/gcc/testsuite/gcc.dg/gimplefe-59.c b/gcc/testsuite/gcc.dg/gimplefe-59.c >>>> new file mode 100644 >>>> index 00000000000..296c088a2a0 >>>> --- /dev/null >>>> +++ b/gcc/testsuite/gcc.dg/gimplefe-59.c >>>> @@ -0,0 +1,16 @@ >>>> +/* { dg-do compile } */ >>>> +/* { dg-options "-fgimple" } */ >>>> + >>>> +/* test REF_REVERSE_STORAGE_ORDER parsing of gimple fe. */ >>>> + >>>> +int __GIMPLE (ssa,guessed_local(1073741824)) >>>> +f2 (void * a, bool b, bool bb) >>>> +{ >>>> + int _1; >>>> + >>>> + __BB(2,guessed_local(1073741824)): >>>> + _1 = __MEM <int> (a_5(D), 1); >>>> + return _1; >>>> + >>>> +} >>>> + >>>> diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc >>>> index bd60e5c15c1..316945c00ed 100644 >>>> --- a/gcc/tree-pretty-print.cc >>>> +++ b/gcc/tree-pretty-print.cc >>>> @@ -2075,6 +2075,12 @@ dump_mem_ref (pretty_printer *pp, tree node, int spc, dump_flags_t flags) >>>> spc, flags | TDF_SLIM, false); >>>> } >>>> } >>>> + if (TREE_CODE (node) == MEM_REF >>>> + && REF_REVERSE_STORAGE_ORDER (node)) >>>> + { >>>> + pp_string (pp, ", "); >>>> + pp_decimal_int (pp, REF_REVERSE_STORAGE_ORDER (node)); >>>> + } >>>> if (MR_DEPENDENCE_CLIQUE (node) != 0) >>>> { >>>> pp_string (pp, ", "); >>>> -- >>>> 2.43.0 >>>>