Re: [RFA for x86] Don't include subst attributes in "@" md helpers
On Thu, Dec 19, 2024 at 12:01 AM Richard Sandiford wrote: > > In a later patch, I need to add "@" to a pattern that uses subst > iterators. This combination is problematic for two reasons: > > (1) define_substs are applied and filtered at a later stage than the > handling of "@" patterns, so that the handling of "@" patterns > doesn't know which subst variants are valid and which will later be > dropped. Just adding a "@" therefore triggers a build error due to > references to non-existent patterns. > > (2) Currently, the code will treat a single "@" pattern as contributing > to a single set of overloaded functions. These overloaded functions > will have an integer argument for every subst iterator. For example, > the vczle and vczbe in: > > "@aarch64_rev" > > are subst iterators, and so currently we'd try to generate a > single set of overloads that take four arguments: one for rev_op, > one for the mode, one for vczle, and one for vczbe. The gen_* > and maybe_gen_* functions will also have one rtx argument for > each operand in the original pattern. > > This model doesn't really make sense for define_substs, since > define_substs are allowed to add extra operands to an instruction. > The number of rtx operands to the generators would then be > incorrect. > > I think a more sensible way of handling define_substs would be to > apply them first (and thus expand things like and > above) and then apply "@". However, that's a relatively invasive > change and not suitable for stage 3. > > This patch instead skips over subst iterators and restricts "@" > overload handling to the cases where no define_subst is applied. > I looked through all uses of "@" names in target code and there > seemed to be only one current use of "@" with define_substs, > in x86 vector code. The current behaviour seemed to be unwanted there, > and the x86 code was having to work around it. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. I think the x86 > changes are bordering on obvious, but just in case: ok for those? LGTM for x86 part. > > Thanks, > Richard > > > gcc/ > * read-rtl.cc (md_reader::handle_overloaded_name): Don't add > arguments for uses of subst attributes. > (apply_iterators): Only add instructions to an overloaded helper > if they use the default subst iterator values. > * doc/md.texi: Update documentation accordingly. > * config/i386/i386-expand.cc (expand_vec_perm_broadcast_1): Update > accordingly. > --- > gcc/config/i386/i386-expand.cc | 4 ++-- > gcc/doc/md.texi| 5 + > gcc/read-rtl.cc| 18 ++ > 3 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc > index 6d25477841a..7f1dcd0937b 100644 > --- a/gcc/config/i386/i386-expand.cc > +++ b/gcc/config/i386/i386-expand.cc > @@ -23884,7 +23884,7 @@ expand_vec_perm_broadcast_1 (struct expand_vec_perm_d > *d) >if (d->testing_p) > return true; > > - rtx (*gen_interleave) (machine_mode, int, rtx, rtx, rtx); > + rtx (*gen_interleave) (machine_mode, rtx, rtx, rtx); >if (elt >= nelt2) > { > gen_interleave = gen_vec_interleave_high; > @@ -23895,7 +23895,7 @@ expand_vec_perm_broadcast_1 (struct expand_vec_perm_d > *d) >nelt2 /= 2; > >dest = gen_reg_rtx (vmode); > - emit_insn (gen_interleave (vmode, 1, dest, op0, op0)); > + emit_insn (gen_interleave (vmode, dest, op0, op0)); > >vmode = V4SImode; >op0 = gen_lowpart (vmode, dest); > diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi > index f0b63a144ad..a997ab07f21 100644 > --- a/gcc/doc/md.texi > +++ b/gcc/doc/md.texi > @@ -12359,4 +12359,9 @@ output and three inputs). This combination would > produce separate > each operand count, but it would still produce a single > @samp{maybe_code_for_@var{name}} and a single @samp{code_for_@var{name}}. > > +Currently, these @code{@@} patterns only take into account patterns for > +which no @code{define_subst} has been applied (@pxref{Define Subst}). > +Any @samp{<@dots{}>} placeholders that refer to subst attributes > +(@pxref{Subst Iterators}) are ignored. > + > @end ifset > diff --git a/gcc/read-rtl.cc b/gcc/read-rtl.cc > index 195f78bd5e1..49a5306254b 100644 > --- a/gcc/read-rtl.cc > +++ b/gcc/read-rtl.cc > @@ -814,9 +814,14 @@ md_reader::handle_overloaded_name (rtx original, > vec *iterators) > pending_underscore_p = false; > } > > - /* Record an argument for ITERATOR. */ > - iterators->safe_push (iterator); > - tmp_oname.arg_types.safe_push (iterator->group->type); > + /* Skip define_subst iterators, since define_substs are allowed to > +add new match_operands in their output templates. */ > + if (iterator->group != &substs) > + { > + /* Record an argument fo
Re: [RFC PATCH v2] RISC-V:Fix th.vsetvli generates from vext_x_v with wrong operand
On Mon, Dec 23, 2024 at 2:52 PM wrote: > > From: Yunze Zhu > > Fix a bug th.vsetvli generates from vext_x_v with an imm operand, > which reports illegal operand. This patch fix this by replacing > imm operand with reg operand in th.vsetvli. > > gcc/ChangeLog: > > * config/riscv/riscv-vsetvl.cc: > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/rvv/xtheadvector/vext_x_v.c: New test. > --- > gcc/config/riscv/riscv-vsetvl.cc| 2 +- > .../riscv/rvv/xtheadvector/vext_x_v.c | 17 + > 2 files changed, 18 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vext_x_v.c > > diff --git a/gcc/config/riscv/riscv-vsetvl.cc > b/gcc/config/riscv/riscv-vsetvl.cc > index 720d52964c1c..e6ee074c19fb 100644 > --- a/gcc/config/riscv/riscv-vsetvl.cc > +++ b/gcc/config/riscv/riscv-vsetvl.cc > @@ -1186,7 +1186,7 @@ public: > set the value of avl to (const_int 0) so that VSETVL PASS will > insert vsetvl correctly.*/ > if (!get_avl ()) > - avl = GEN_INT (0); > + avl = TARGET_XTHEADVECTOR ? gen_rtx_REG (Pmode, 0) : GEN_INT (0); I feel the semantic has changed here, I checked v-spec 0.7.1 it says use x0 here means VLMAX rather than real 0, although it's fine to `th.vext.x.v`, but it just does not seem the right place to fix? But don't treat my comment as a blocker since it's vendor specific logic so it's up to you guys :P > rtx sew = gen_int_mode (get_sew (), Pmode); > rtx vlmul = gen_int_mode (get_vlmul (), Pmode); > rtx ta = gen_int_mode (get_ta (), Pmode); > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vext_x_v.c > b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vext_x_v.c > new file mode 100644 > index ..be5847727cac > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vext_x_v.c > @@ -0,0 +1,17 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcxtheadvector -mabi=lp64d -O3 " } */ > +#include > + > +int64_t f1 (void * in) > +{ > +vint64m1_t v = __riscv_th_vlb_v_i64m1 (in, 2); > +vint32m1_t v2 = __riscv_th_vlb_v_i32m1 (in, 2); > +int64_t i1 = __riscv_th_vext_x_v_i64m1_i64(v, 2); > +int32_t i2 = __riscv_th_vext_x_v_i32m1_i32(v2, 2); > +int64_t i = i1 + (int64_t)i2; > +return i; > +} > + > +/* { dg-final { scan-assembler-times "th.vsetvli zero,zero,e32,m1" 1 } > } */ > +/* { dg-final { scan-assembler-times "th.vsetvli zero,zero,e64,m1" 1 } > } */ > +/* { dg-final { scan-assembler-not "th.vsetvli zero,0" } } */ > -- > 2.25.1 >
[Patch, fortran] PR116254/118059 -[15 Regression] Bugs triggered by class_transformational_1/2.f90
Hi All, These bugs were tricky to find because neither were detected by regression testing on my platform. PR116254: This bug was sporadic even where the regression was detected. Richard Sandiford found "Conditional jump or move depends on uninitialised value" errors in the library, triggered by the test for for spread in class_transformational_2.f90. Mea culpa, I had failed to notice this. It turned out that, alone of the rank changing intrinsic functions, spread was going through the wrong code path. This is fixed by the explicit condition in the patch. Richard also noted that spread results are being copied from uninitialised memory. This was not quite correct because the library function was doing the job on detection of the NULL descriptor data field. Nevertheless, I have refactored slightly to ensure that the temporary descriptor has its dtype field set correctly before the class data is pointed at it. Valgrind shows both class_transformational tests to now be as clean as a whistle. PR118059: The reporter found, with an ubsan instrumented gcc, that class_transformational_1.f90 was generating an invalid value for type 'expr_t' in gcc/fortran/trans-expr.cc. The offending statement is; B = reshape (chr, [5, 1, 2]), where 'B' is unlimited polymorphic. This requires a trivial fix since the assignment of a character array valued function to an unlimited polymorphic variable unconditionally requires a temporary. Regression tests without errors - OK for mainline? Seasons greetings to one and all. Paul Change.Logs Description: Binary data diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 82a2ae1f747..3132443b4d9 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -1632,9 +1632,20 @@ gfc_trans_create_temp_array (stmtblock_t * pre, stmtblock_t * post, gfc_ss * ss, tree class_data; tree dtype; gfc_expr *expr1 = fcn_ss ? fcn_ss->info->expr : NULL; + bool rank_changer; + + /* Pick out these transformational functions because they change the rank + or shape of the first argument. This requires that the class type be + changed, the dtype updating and the correct rank used. */ + rank_changer = expr1 && expr1->expr_type == EXPR_FUNCTION + && expr1->value.function.isym + && (expr1->value.function.isym->id == GFC_ISYM_RESHAPE + || expr1->value.function.isym->id == GFC_ISYM_SPREAD + || expr1->value.function.isym->id == GFC_ISYM_PACK + || expr1->value.function.isym->id == GFC_ISYM_UNPACK); /* Create a class temporary for the result using the lhs class object. */ - if (class_expr != NULL_TREE) + if (class_expr != NULL_TREE && !rank_changer) { tmp = gfc_create_var (TREE_TYPE (class_expr), "ctmp"); gfc_add_modify (pre, tmp, class_expr); @@ -1672,33 +1683,29 @@ gfc_trans_create_temp_array (stmtblock_t * pre, stmtblock_t * post, gfc_ss * ss, elemsize = gfc_evaluate_now (elemsize, pre); } - /* Assign the new descriptor to the _data field. This allows the - vptr _copy to be used for scalarized assignment since the class - temporary can be found from the descriptor. */ class_data = gfc_class_data_get (tmp); - tmp = fold_build1_loc (input_location, VIEW_CONVERT_EXPR, - TREE_TYPE (desc), desc); - gfc_add_modify (pre, class_data, tmp); - if (expr1 && expr1->expr_type == EXPR_FUNCTION - && expr1->value.function.isym - && (expr1->value.function.isym->id == GFC_ISYM_RESHAPE - || expr1->value.function.isym->id == GFC_ISYM_UNPACK)) + if (rank_changer) { /* Take the dtype from the class expression. */ dtype = gfc_conv_descriptor_dtype (gfc_class_data_get (class_expr)); - tmp = gfc_conv_descriptor_dtype (class_data); + tmp = gfc_conv_descriptor_dtype (desc); gfc_add_modify (pre, tmp, dtype); - /* Transformational functions reshape and reduce can change the rank. */ - if (fcn_ss && fcn_ss->info && fcn_ss->info->class_container) - { - tmp = gfc_conv_descriptor_rank (class_data); - gfc_add_modify (pre, tmp, - build_int_cst (TREE_TYPE (tmp), ss->loop->dimen)); - fcn_ss->info->class_container = NULL_TREE; - } + /* These transformational functions change the rank. */ + tmp = gfc_conv_descriptor_rank (desc); + gfc_add_modify (pre, tmp, + build_int_cst (TREE_TYPE (tmp), ss->loop->dimen)); + fcn_ss->info->class_container = NULL_TREE; } + + /* Assign the new descriptor to the _data field. This allows the + vptr _copy to be used for scalarized assignment since the class + temporary can be found from the descriptor. */ + tmp = fold_build1_loc (input_location, VIEW_CONVERT_EXPR, + TREE_TYPE (desc), desc); + gfc_add_modify (pre, class_data, tmp); + /* Point desc to the class _data field. */ desc = class_data; } diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 44a50c0edb1..83474ea6770 100644 --- a/gcc/fort
Re: [PATCH v4] arm: [MVE intrinsics] Fix support for predicate constants [PR target/114801]
On Mon, 23 Dec 2024 at 05:58, Andrew Pinski wrote: > > On Fri, Dec 13, 2024 at 6:31 AM Christophe Lyon > wrote: > > > > On Tue, 10 Dec 2024 at 13:14, Richard Earnshaw (lists) > > wrote: > > > > > > On 09/12/2024 21:11, Christophe Lyon wrote: > > > > In this PR, we have to handle a case where MVE predicates are supplied > > > > as a const_int, where individual predicates have illegal boolean > > > > values (such as 0xc for a 4-bit boolean predicate). To avoid the ICE, > > > > fix the constant (any non-zero value is converted to all 1s) and emit > > > > a warning. > > > > > > > > On MVE, V8BI and V4BI multi-bit masks are interpreted byte-by-byte at > > > > instruction level, but end-users should describe lanes rather than > > > > bytes (so all bytes of a true-predicated lane should be '1'), see the > > > > section on MVE intrinsics in the Arm ACLE specification. > > > > > > > > Since force_lowpart_subreg cannot handle const_int (because they have > > > > VOID mode), > > > > use gen_lowpart on them, force_lowpart_subreg otherwise. > > > > > > > > 2024-11-20 Christophe Lyon > > > > Jakub Jelinek > > > > > > > > PR target/114801 > > > > gcc/ > > > > * config/arm/arm-mve-builtins.cc > > > > (function_expander::add_input_operand): Handle CONST_INT > > > > predicates. > > > > > > > > gcc/testsuite/ > > > > * gcc.target/arm/mve/pr108443.c: Update predicate constant. > > > > * gcc.target/arm/mve/pr108443-run.c: Likewise. > > > > * gcc.target/arm/mve/pr114801.c: New test. > > > > > > Thanks, that looks much better. OK, assuming no regressions. > > > > > > > Indeed, thanks for your suggestions. > > > > OK to backport to gcc-14 after a while? > > Except force_lowpart_subreg does not exist on the 14 branch. > Did you test it there before pushing it? > Oops sorry for the breakage. I've reverted it, I shouldn't have rushed before holidays. I'll check better when I'm back. Thanks, Christophe > Thanks, > Andrew Pinski > > > > > Thanks, > > > > Christophe > > > > > > > R. > > > > > > > --- > > > > gcc/config/arm/arm-mve-builtins.cc| 32 ++- > > > > .../gcc.target/arm/mve/pr108443-run.c | 2 +- > > > > gcc/testsuite/gcc.target/arm/mve/pr108443.c | 4 +- > > > > gcc/testsuite/gcc.target/arm/mve/pr114801.c | 39 +++ > > > > 4 files changed, 73 insertions(+), 4 deletions(-) > > > > create mode 100644 gcc/testsuite/gcc.target/arm/mve/pr114801.c > > > > > > > > diff --git a/gcc/config/arm/arm-mve-builtins.cc > > > > b/gcc/config/arm/arm-mve-builtins.cc > > > > index 8570e18fd96..3c3d30bd0de 100644 > > > > --- a/gcc/config/arm/arm-mve-builtins.cc > > > > +++ b/gcc/config/arm/arm-mve-builtins.cc > > > > @@ -2358,7 +2358,37 @@ function_expander::add_input_operand (insn_code > > > > icode, rtx x) > > > > mode = GET_MODE (x); > > > > } > > > > else if (VALID_MVE_PRED_MODE (mode)) > > > > -x = gen_lowpart (mode, x); > > > > +{ > > > > + if (CONST_INT_P (x)) > > > > + { > > > > + if (mode == V8BImode || mode == V4BImode) > > > > + { > > > > + /* In V8BI or V4BI each element has 2 or 4 bits, if those > > > > bits > > > > + aren't all the same, gen_lowpart might ICE. > > > > Canonicalize all > > > > + the 2 or 4 bits to all ones if any of them is non-zero. > > > > V8BI > > > > + and V4BI multi-bit masks are interpreted byte-by-byte at > > > > + instruction level, but such constants should describe > > > > lanes, > > > > + rather than bytes. See the section on MVE intrinsics in > > > > the > > > > + Arm ACLE specification. */ > > > > + unsigned HOST_WIDE_INT xi = UINTVAL (x); > > > > + xi |= ((xi & 0x) << 1) | ((xi & 0x) >> 1); > > > > + if (mode == V4BImode) > > > > + xi |= ((xi & 0x) << 2) | ((xi & 0x) >> 2); > > > > + if (xi != UINTVAL (x)) > > > > + warning_at (location, 0, "constant predicate argument %d" > > > > + " (%wx) does not map to %d lane numbers," > > > > + " converted to %wx", > > > > + opno, UINTVAL (x) & 0x, > > > > + mode == V8BImode ? 8 : 4, > > > > + xi & 0x); > > > > + > > > > + x = gen_int_mode (xi, HImode); > > > > + } > > > > + x = gen_lowpart (mode, x); > > > > + } > > > > + else > > > > + x = force_lowpart_subreg (mode, x, GET_MODE (x)); > > > > +} > > > > > > > > m_ops.safe_grow (m_ops.length () + 1, true); > > > > create_input_operand (&m_ops.last (), x, mode); > > > > diff --git a/gcc/testsuite/gcc.target/arm/mve/pr108443-run.c > > > > b/gcc/testsuite/gcc.target/arm/mve/pr108443-run.c > > > > index cb4b45bd305..b894f019b8b 100644 > > > > --- a/gcc/testsuite/gcc.target/arm/m
Re: [PATCH] vect: Do not use partial vectors when emulating vectors [PR116351].
> I don't quite understand - you are checking loop_vinfo->vector_mode, but > how can you be sure no chosen vector uses a !VECTOR_MODE_P? It seems > fragile to rely on (it might work in this case), instead when any > !VECTOR_MODE_P needs a 'len' we should reject it - so why does this > not happen here? It appears we detect an over-widening pattern and set the initial ("natural"?) vector mode there to DImode. Afterwards we find a vectorizable_reduction that does not check for partial vector usage (no single def-use cycle nor fold-left reduction). As there are no loads/stores we never reach check_load_store_for_partial_vectors which disables partial vectors. Should the check then rather be in vectorizable_reduction? -- Regards Robin
Re: [PATCH] vect: Do not use partial vectors when emulating vectors [PR116351].
> Am 23.12.2024 um 10:57 schrieb Robin Dapp : > > >> >> I don't quite understand - you are checking loop_vinfo->vector_mode, but >> how can you be sure no chosen vector uses a !VECTOR_MODE_P? It seems >> fragile to rely on (it might work in this case), instead when any >> !VECTOR_MODE_P needs a 'len' we should reject it - so why does this >> not happen here? > > It appears we detect an over-widening pattern and set the initial ("natural"?) > vector mode there to DImode. And that pattern is the Reduction stmt only? > Afterwards we find a vectorizable_reduction that > does not check for partial vector usage (no single def-use cycle nor fold-left > reduction). > > As there are no loads/stores we never reach > check_load_store_for_partial_vectors which disables partial vectors. > Should the check then rather be in vectorizable_reduction? Unless the stmt needs not be predicated yes (but most reduction stmts are covered in their vectorizable_* routines) Richard > > -- > Regards > Robin >
[PATCH] libcc1: Fix tags generation target
'make tags' currently fails for libcc1 with this: *** No rule to make target `marshall-c.hh', needed by `tags-am'. Stop. The problem is that while marshall-c.hh has been removed via r12-454-g25d1a6ecdc443f, it's still part of the libcc1_la_SOURCES variable, hence the 'tags' target has a dependency on it. This patch simply removes the marshall_c_source variable, that should be empty. Successfully tested with 'make tags' at the root of the build directory. libcc1/ChangeLog: * Makefile.am: Remove reference to deleted marshall-c.h. * Makefile.in: Regenerate. --- libcc1/Makefile.am | 5 ++--- libcc1/Makefile.in | 9 - 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/libcc1/Makefile.am b/libcc1/Makefile.am index b592bc8645f..062f4a74cc2 100644 --- a/libcc1/Makefile.am +++ b/libcc1/Makefile.am @@ -51,12 +51,11 @@ endif shared_source = callbacks.cc callbacks.hh connection.cc connection.hh \ marshall.cc marshall.hh rpc.hh status.hh -marshall_c_source = marshall-c.hh marshall_cxx_source = marshall-cp.hh libcc1plugin_la_LDFLAGS = -module -export-symbols $(srcdir)/libcc1plugin.sym libcc1plugin_la_SOURCES = libcc1plugin.cc context.cc context.hh \ - $(shared_source) $(marshall_c_source) + $(shared_source) libcc1plugin.lo_CPPFLAGS = $(CPPFLAGS_FOR_C) libcc1plugin_la_LIBADD = $(libiberty) libcc1plugin_la_DEPENDENCIES = $(libiberty_dep) @@ -78,7 +77,7 @@ LTLDFLAGS = $(shell $(SHELL) $(top_srcdir)/../libtool-ldflags $(LDFLAGS)) libcc1_la_LDFLAGS = -module -export-symbols $(srcdir)/libcc1.sym libcc1_la_SOURCES = findcomp.cc libcc1.cc libcp1.cc \ compiler.cc compiler.hh names.cc names.hh $(shared_source) \ - $(marshall_c_source) $(marshall_cxx_source) + $(marshall_cxx_source) libcc1_la_LIBADD = $(libiberty) libcc1_la_DEPENDENCIES = $(libiberty_dep) libcc1_la_LINK = $(LIBTOOL) --tag=CXX $(AM_LIBTOOLFLAGS) \ diff --git a/libcc1/Makefile.in b/libcc1/Makefile.in index f8f590d71e9..9d56a8323b0 100644 --- a/libcc1/Makefile.in +++ b/libcc1/Makefile.in @@ -145,11 +145,11 @@ LTLIBRARIES = $(cc1lib_LTLIBRARIES) $(plugin_LTLIBRARIES) am__objects_1 = callbacks.lo connection.lo marshall.lo am__objects_2 = am_libcc1_la_OBJECTS = findcomp.lo libcc1.lo libcp1.lo compiler.lo \ - names.lo $(am__objects_1) $(am__objects_2) $(am__objects_2) + names.lo $(am__objects_1) $(am__objects_2) libcc1_la_OBJECTS = $(am_libcc1_la_OBJECTS) @ENABLE_PLUGIN_TRUE@am_libcc1_la_rpath = -rpath $(cc1libdir) am_libcc1plugin_la_OBJECTS = libcc1plugin.lo context.lo \ - $(am__objects_1) $(am__objects_2) + $(am__objects_1) libcc1plugin_la_OBJECTS = $(am_libcc1plugin_la_OBJECTS) @ENABLE_PLUGIN_TRUE@am_libcc1plugin_la_rpath = -rpath $(plugindir) am_libcp1plugin_la_OBJECTS = libcp1plugin.lo context.lo \ @@ -403,11 +403,10 @@ cc1libdir = $(libdir)/$(libsuffix) shared_source = callbacks.cc callbacks.hh connection.cc connection.hh \ marshall.cc marshall.hh rpc.hh status.hh -marshall_c_source = marshall-c.hh marshall_cxx_source = marshall-cp.hh libcc1plugin_la_LDFLAGS = -module -export-symbols $(srcdir)/libcc1plugin.sym libcc1plugin_la_SOURCES = libcc1plugin.cc context.cc context.hh \ - $(shared_source) $(marshall_c_source) + $(shared_source) libcc1plugin.lo_CPPFLAGS = $(CPPFLAGS_FOR_C) libcc1plugin_la_LIBADD = $(libiberty) @@ -431,7 +430,7 @@ LTLDFLAGS = $(shell $(SHELL) $(top_srcdir)/../libtool-ldflags $(LDFLAGS)) libcc1_la_LDFLAGS = -module -export-symbols $(srcdir)/libcc1.sym libcc1_la_SOURCES = findcomp.cc libcc1.cc libcp1.cc \ compiler.cc compiler.hh names.cc names.hh $(shared_source) \ - $(marshall_c_source) $(marshall_cxx_source) + $(marshall_cxx_source) libcc1_la_LIBADD = $(libiberty) libcc1_la_DEPENDENCIES = $(libiberty_dep) -- 2.44.0
[PATCH] RISC-V: Fix code gen for reduction with length 0 [PR118182]
--- gcc/config/riscv/autovec-opt.md | 16 ++-- gcc/config/riscv/autovec.md | 30 +++ gcc/config/riscv/riscv-v.cc | 4 +- gcc/config/riscv/vector-iterators.md | 46 +++ gcc/config/riscv/vector.md | 118 +++ 5 files changed, 189 insertions(+), 25 deletions(-) diff --git a/gcc/config/riscv/autovec-opt.md b/gcc/config/riscv/autovec-opt.md index 645dc53d868..4704bf342e3 100644 --- a/gcc/config/riscv/autovec-opt.md +++ b/gcc/config/riscv/autovec-opt.md @@ -810,7 +810,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP, operands, CONST0_RTX (mode)); DONE; @@ -829,7 +829,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, riscv_vector::REDUCE_OP_FRM_DYN, operands, CONST0_RTX (mode)); @@ -850,7 +850,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED_AV, riscv_vector::REDUCE_OP_FRM_DYN, operands, operands[2]); DONE; @@ -878,7 +878,7 @@ else { rtx ops[] = {operands[0], operands[2], operands[3], operands[4]}; - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED_AV, riscv_vector::REDUCE_OP_M_FRM_DYN, ops, operands[1]); } @@ -1226,7 +1226,7 @@ { rtx ops[] = {operands[0], operands[2], operands[1], gen_int_mode (GET_MODE_NUNITS (mode), Pmode)}; - riscv_vector::expand_reduction (, + riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP_M, ops, CONST0_RTX (mode)); DONE; @@ -1281,7 +1281,7 @@ [(const_int 0)] { rtx ops[] = {operands[0], operands[3], operands[1], operands[2]}; - riscv_vector::expand_reduction (, + riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP_M, ops, CONST0_RTX (mode)); DONE; @@ -1317,7 +1317,7 @@ { rtx ops[] = {operands[0], operands[2], operands[1], gen_int_mode (GET_MODE_NUNITS (mode), Pmode)}; - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, riscv_vector::REDUCE_OP_M_FRM_DYN, ops, CONST0_RTX (mode)); DONE; @@ -1372,7 +1372,7 @@ [(const_int 0)] { rtx ops[] = {operands[0], operands[3], operands[1], operands[2]}; - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, riscv_vector::REDUCE_OP_M_FRM_DYN, ops, CONST0_RTX (mode)); DONE; diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md index 8d30ab22186..4432b7d1d05 100644 --- a/gcc/config/riscv/autovec.md +++ b/gcc/config/riscv/autovec.md @@ -2097,7 +2097,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (UNSPEC_REDUC_SUM, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (UNSPEC_REDUC_SUM_AV, riscv_vector::REDUCE_OP, operands, CONST0_RTX (mode)); DONE; } @@ -2110,7 +2110,7 @@ { int prec = GET_MODE_PRECISION (mode); rtx min = immed_wide_int_const (wi::min_value (prec, SIGNED), mode); - riscv_vector::expand_reduction (UNSPEC_REDUC_MAX, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (UNSPEC_REDUC_MAX_AV, riscv_vector::REDUCE_OP, operands, min); DONE; }) @@ -2120,7 +2120,7 @@ (match_operand:V_VLSI 1 "register_operand")] "TARGET_VECTOR" { - riscv_vector::expand_reduction (UNSPEC_REDUC_MAXU, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (UNSPEC_REDUC_MAXU_AV, riscv_vector::REDUCE_OP, operands, CONST0_RTX (mode)); DONE; }) @@ -2132,7 +2132,7 @@ { int prec = GET_MODE_PRECISION (mode); rtx max = immed_wide_int_const (wi::max_value (prec, SIGNED), mode); - riscv_vector::expand_reduction (UNSPEC_REDUC_MIN, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (UNSPEC_REDUC_MIN_AV, riscv_vector::REDUCE_OP, operands, max); DONE; }) @@ -2144,7 +2144,7 @@ { int prec = GET_MODE_PRECISION (mode); rtx max = immed_wide_int_const (wi::max_value (prec, UNSIGNED), mode); - riscv_vector
Re: [PATCH] RISC-V: Fix code gen for reduction with length 0 [PR118182]
> +;; Integer Reduction (vred(sum|maxu|max|minu|min|and|or|xor).vs), but for > auto vectorizer > +(define_insn "@pred_av_" > + [(set (match_operand: 0 "register_operand" "=vr") > + (unspec: > + [(unspec: > + [(match_operand: 1 "vector_mask_operand" "vmWc1") > + (match_operand 5 "vector_length_operand" " rK") > + (match_operand 6 "const_int_operand" "i") > + (match_operand 7 "const_int_operand" "i") > + (reg:SI VL_REGNUM) > + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE) > +(unspec: [ > + (match_operand:V_VLSI3 "register_operand" " vr") > + (match_operand: 4 "register_operand" " vr") > + ] ANY_REDUC_AV) > +(match_operand: 2 "vector_merge_operand" " vu")] > UNSPEC_REDUC))] > + "TARGET_VECTOR" > + "v.vs\t%0,%3,%4%p1" > + [(set_attr "type" "vired") > + (set_attr "mode" "")]) Shouldn't operand 4 have a "0" constraint here and in the following two changed patterns? Or am I missing something? I'm not yet fully convinced we should go with this approach over the one with the additional vsetvl (that might get merged with a preceding vsetvl and therefore free). -- Regards Robin
[PATCH] RISC-V: Move fortran testcase to gfortran.target
gcc/testsuite/ChangeLog: * gcc.target/riscv/rvv/fortran/pr111395.f90: Move this file to... * gfortran.target/riscv/rvv/pr111395.f90: ...here. * gcc.target/riscv/rvv/fortran/pr111566.f90: Move this file to... * gfortran.target/riscv/rvv/pr111566.f90: ...here. * gcc.target/riscv/rvv/rvv-fortran.exp: Move this file to... * gfortran.target/riscv/rvv/rvv.exp: ...here. --- NOTE: This patch will commit once CI pass. --- .../rvv/fortran => gfortran.target/riscv/rvv}/pr111395.f90 | 0 .../rvv/fortran => gfortran.target/riscv/rvv}/pr111566.f90 | 0 .../rvv/rvv-fortran.exp => gfortran.target/riscv/rvv/rvv.exp} | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename gcc/testsuite/{gcc.target/riscv/rvv/fortran => gfortran.target/riscv/rvv}/pr111395.f90 (100%) rename gcc/testsuite/{gcc.target/riscv/rvv/fortran => gfortran.target/riscv/rvv}/pr111566.f90 (100%) rename gcc/testsuite/{gcc.target/riscv/rvv/rvv-fortran.exp => gfortran.target/riscv/rvv/rvv.exp} (93%) diff --git a/gcc/testsuite/gcc.target/riscv/rvv/fortran/pr111395.f90 b/gcc/testsuite/gfortran.target/riscv/rvv/pr111395.f90 similarity index 100% rename from gcc/testsuite/gcc.target/riscv/rvv/fortran/pr111395.f90 rename to gcc/testsuite/gfortran.target/riscv/rvv/pr111395.f90 diff --git a/gcc/testsuite/gcc.target/riscv/rvv/fortran/pr111566.f90 b/gcc/testsuite/gfortran.target/riscv/rvv/pr111566.f90 similarity index 100% rename from gcc/testsuite/gcc.target/riscv/rvv/fortran/pr111566.f90 rename to gcc/testsuite/gfortran.target/riscv/rvv/pr111566.f90 diff --git a/gcc/testsuite/gcc.target/riscv/rvv/rvv-fortran.exp b/gcc/testsuite/gfortran.target/riscv/rvv/rvv.exp similarity index 93% rename from gcc/testsuite/gcc.target/riscv/rvv/rvv-fortran.exp rename to gcc/testsuite/gfortran.target/riscv/rvv/rvv.exp index a01c3d09170..d85f58336e1 100644 --- a/gcc/testsuite/gcc.target/riscv/rvv/rvv-fortran.exp +++ b/gcc/testsuite/gfortran.target/riscv/rvv/rvv.exp @@ -39,7 +39,7 @@ dg-init # Main loop. gfortran-dg-runtest [lsort \ - [glob -nocomplain $srcdir/$subdir/fortran/*.\[fF\]{,90,95,03,08} ] ] "" "" + [glob -nocomplain $srcdir/$subdir/*.\[fF\]{,90,95,03,08} ] ] "" "" # All done. dg-finish -- 2.34.1
Re: [PATCH] testsuite/gcc.dg/memcmp-1.c: Cut down a factor of 7 for simulators
On 12/22/24 6:19 PM, Hans-Peter Nilsson wrote: I could do it just for target mmix, but that wouldn't help other simulator targets. Using different primes is deliberate. Ok to commit? -- >8 -- Running tests in parallel on my 4.5y+ old laptop made this test time out: the test itself runs in 9m20s, the timeout being 10 minutes with the 2x factor. That's a bit too close. This commit does to the base test a similar change as was done for gcc.dg/torture/inline-mem-cpy-1.c in commit r14-8188-g6eca0d23b7ea84; or IOW cut it down a factor of 7 (r14-8188 was by a factor of 11). * gcc.dg/memcmp-1.c: Pass -DRUN_FRACTION=7 when testing in a simulator. OK. I think you should consider similar patches pre-approved. jeff
Re: [r15-6415 Regression] FAIL: gfortran.dg/coarray_lib_comm_1.f90 -Os scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 on Linu
Hi Andre, It looks good to me. Thanks Paul On Mon, 23 Dec 2024 at 14:58, Andre Vehreschild wrote: > Hi all, > > I did an ooppsie with the patch for 107635. Attached is a patch that fixes > this. Essentially the regexp was to complicated for what was needed. So > now we > just count the number of occurrences of caf_get_by_ct, which has to be > four. > > Regtests ok on x86_64-pc-linux-gnu. Ok for mainline? > > Regards, > Andre > > On Mon, 23 Dec 2024 04:52:50 +0800 > "haochen.jiang" wrote: > > > On Linux/x86_64, > > > > 586477d67bf2e320e8ec41f82b194259c1dcc43a is the first bad commit > > commit 586477d67bf2e320e8ec41f82b194259c1dcc43a > > Author: Andre Vehreschild > > Date: Fri Dec 6 08:57:34 2024 +0100 > > > > Fortran: Replace getting of coarray data with accessor-based version. > > [PR107635] > > > > caused > > > > FAIL: gfortran.dg/coarray_lib_comm_1.f90 -O0 scan-tree-dump-times > > original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned > > long\\) atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 > -O1 > > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., > 0B, > > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 FAIL: > > gfortran.dg/coarray_lib_comm_1.f90 -O2 scan-tree-dump-times original > > "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) > > atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 -O3 > > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer > -finline-functions > > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., > 0B, > > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 FAIL: > > gfortran.dg/coarray_lib_comm_1.f90 -O3 -g scan-tree-dump-times > original > > "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) > > atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 -Os > > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., > 0B, > > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 > > > > with GCC configured with > > > > ../../gcc/configure > > --prefix=/export/users/haochenj/src/gcc-bisect/master/master/r15-6415/usr > > --enable-clocale=gnu --with-system-zlib --with-demangler-in-ld > > --with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet > --without-isl > > --enable-libmpx x86_64-linux --disable-bootstrap > > > > To reproduce: > > > > $ cd {build_dir}/gcc && make check > > RUNTESTFLAGS="dg.exp=gfortran.dg/coarray_lib_comm_1.f90 > > --target_board='unix{-m32}'" $ cd {build_dir}/gcc && make check > > RUNTESTFLAGS="dg.exp=gfortran.dg/coarray_lib_comm_1.f90 > > --target_board='unix{-m32\ -march=cascadelake}'" > > > > (Please do not reply to this email, for question about this report, > contact > > me at haochen dot jiang at intel.com.) (If you met problems with > cascadelake > > related, disabling AVX512F in command line might save that.) (However, > please > > make sure that there is no potential problems with AVX512.) > > > -- > Andre Vehreschild * Email: vehre ad gmx dot de >
Re: [PATCH v4 6/7] OpenMP: Fortran front-end support for dispatch + adjust_args
Hi Tobias, Replying to your last two messages here and attaching revised patches. On 16/12/2024 22:34, Tobias Burnus wrote: I have not looked in depth at the patch, but managed to write C-ism code, which caused a segfault (due to a missing "call"), after gfortran issued a reasonable error. Can you fix it and, just to make sure, add it as another testcase? foo.f90:18:7: 18 | g(a,b,c) | 1 Error: ‘g’ at (1) is not a variable foo.f90:20:3: 20 | end | 1 Error: Unexpected END statement at (1) Segmentation fault * * * The problem seems to be that during parsing, the location data is NULL, which the error diagnostic does not like: (gdb) p gfc_current_locus $33 = {nextc = 0x0, u = {lb = 0x0, location = 0}} (gdb) p gfc_at_eof() $36 = true and st == ST_NONE. I think the simplest is to check for the last one, and then return early. This will then print: foo.f90:18:7: 18 | g(a,b,c) | 1 Error: ‘g’ at (1) is not a variable foo.f90:20:3: 20 | end | 1 Error: Unexpected END statement at (1) f951: Error: Unexpected end of file in ‘foo.f90’ When the if st is ST_NONE then return check is added: +static gfc_statement +parse_omp_dispatch (void) +{ ... + st = next_statement (); + if (st == ST_NONE) +return st; + if (st == ST_CALL || st == ST_ASSIGNMENT) +accept_statement (st); + else Fixed as suggested. Added testcase. * * * Handling of `adjust_args` across translation units is missing due to PR115271. Namely, https://gcc.gnu.org/PR115271 is about not storing 'declare variant' inside module files; when repeating the decl in an interface, it obviously works as * * * I think the patch is now okay, but I want to re-read it tomorrow - thus, please hold off for a couple of ours. Possibly, others have comments as well :-) * * * TODO: We need to handle 'type(C), dimension(:)' - but I wonder whether that shouldn't be handled as part of 'use_device_addr' and we need to check whether the spec has to be updated. I filed the OpenMP lang-spec Issue #4443. ... and we eventually have to handle 'need_device_addr'/'has_device_addr', but those are follow-up topics. Keeping an eye on the open issue. On 17/12/2024 14:11, Tobias Burnus wrote: Additional comments: Can you hoist the condition out of the loop in: + for (gfc_omp_namelist *n = *head; n != NULL; n = n->next) + if (need_device_ptr_p) + n->u.need_device_ptr = true; Sure. * * * I was about to complain that it didn't handle VALUE + OPTIONAL correctly, but that's a generic gfortran bug (or two): ->https://gcc.gnu.org/PR118080 * * * There is a bug - 'nowait' is not propagated. Trying: !$omp dispatch depend(inout:x) nowait call g(a) !$omp end dispatch gives (-fdump-tree-gimple): #pragma omp taskwait depend(inout:&x) nowait but doing the equivalent !$omp dispatch depend(inout:x) call g(a) !$omp end dispatch nowait gives: #pragma omp taskwait depend(inout:&x) i.e. the 'nowait' got lost. * * * Fixed and added testcase. Similar the original C code, which to my knowledge is now fixed + tested for, there is an issue related to handling nested function calls. I think the attached testcase is fine, but it segfaults unless the default device is the initial device. The problem is that the pointer conversion also happens for the inner function but it should only do so for the outer one. See attached testcase. – I think it can be seen by looking at the dump (and adding an -fdump-tree-gimple + scan test probably won't harm, as not everyone has a GPU and we might implement map as selfmap on APUs). This is actually not specific to the Fortran FE. So I had to modify the middle end and the C++ parser as well. See attached pactches. Otherwise LGTM. Tobias Thanks, -- PAFrom e470fc10269d9bb0a4b263c18f03e289973807c4 Mon Sep 17 00:00:00 2001 From: Paul-Antoine Arras Date: Fri, 24 May 2024 19:13:50 +0200 Subject: [PATCH 1/4] OpenMP: Fortran front-end support for dispatch + adjust_args This patch adds support for the `dispatch` construct and the `adjust_args` clause to the Fortran front-end. Handling of `adjust_args` across translation units is missing due to PR115271. gcc/fortran/ChangeLog: * dump-parse-tree.cc (show_omp_clauses): Handle novariants and nocontext clauses. (show_omp_node): Handle EXEC_OMP_DISPATCH. (show_code_node): Likewise. * frontend-passes.cc (gfc_code_walker): Handle novariants and nocontext. * gfortran.h (enum gfc_statement): Add ST_OMP_DISPATCH. (symbol_attribute): Add omp_declare_variant_need_device_ptr. (gfc_omp_clauses): Add novariants and nocontext. (gfc_omp_declare_variant): Add need_device_ptr_arg_list. (enum gfc_exec_op): Add EXEC_OMP_DISPATCH. * match.h (gfc_match_omp_dispatch): Declare. * openmp.cc (gfc_free_omp_clauses): Free novariants and nocontext clauses. (gfc_free_omp_declare_variant_list): Free need_device_ptr_arg_list namelist. (enum omp_mask2): Add OMP_CLAUSE_NOVARIANTS and OMP_CLAUSE_NOCONTEX
Re: [PATCH] RISC-V: Fix code gen for reduction with length 0 [PR118182]
Oh and I realized I send wrong version which didn't contain the changelog, test case and the description, let me send the v2 first On Mon, Dec 23, 2024 at 11:05 PM Kito Cheng wrote: > > On Mon, Dec 23, 2024 at 10:35 PM Robin Dapp wrote: > > > > > +;; Integer Reduction (vred(sum|maxu|max|minu|min|and|or|xor).vs), but > > > for auto vectorizer > > > +(define_insn "@pred_av_" > > > + [(set (match_operand: 0 "register_operand" > > > "=vr") > > > + (unspec: > > > + [(unspec: > > > + [(match_operand: 1 "vector_mask_operand" "vmWc1") > > > + (match_operand 5 "vector_length_operand" " rK") > > > + (match_operand 6 "const_int_operand" "i") > > > + (match_operand 7 "const_int_operand" "i") > > > + (reg:SI VL_REGNUM) > > > + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE) > > > +(unspec: [ > > > + (match_operand:V_VLSI3 "register_operand" " > > > vr") > > > + (match_operand: 4 "register_operand" " > > > vr") > > > + ] ANY_REDUC_AV) > > > +(match_operand: 2 "vector_merge_operand" " > > > vu")] UNSPEC_REDUC))] > > > + "TARGET_VECTOR" > > > + "v.vs\t%0,%3,%4%p1" > > > + [(set_attr "type" "vired") > > > + (set_attr "mode" "")]) > > > > Shouldn't operand 4 have a "0" constraint here and in the following two > > changed > > patterns? Or am I missing something? > > Good catch, apparently I missed that and spec 2006 didn't catch that > as well...:P > > Will update at V2, > > > I'm not yet fully convinced we should go with this approach over the one > > with > > the additional vsetvl (that might get merged with a preceding vsetvl and > > I guess this might be hard to merge unless the preceding vsetvl come > with a constant VL. > > And here is LLVM's one: > https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/RISCV/rvv/vreductions-int-vp.ll > > I am not trying to say it's the best or only way to resolve...but > that's the only way I found...so brainstorming start! :P > > > therefore free). > > > > > > > -- > > Regards > > Robin > >
[PATCH v2] RISC-V: Fix code gen for reduction with length 0 [PR118182]
--- gcc/config/riscv/autovec-opt.md | 16 ++-- gcc/config/riscv/autovec.md | 30 +++ gcc/config/riscv/riscv-v.cc | 4 +- gcc/config/riscv/vector-iterators.md | 46 +++ gcc/config/riscv/vector.md | 118 +++ 5 files changed, 189 insertions(+), 25 deletions(-) diff --git a/gcc/config/riscv/autovec-opt.md b/gcc/config/riscv/autovec-opt.md index 645dc53d868..4704bf342e3 100644 --- a/gcc/config/riscv/autovec-opt.md +++ b/gcc/config/riscv/autovec-opt.md @@ -810,7 +810,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP, operands, CONST0_RTX (mode)); DONE; @@ -829,7 +829,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, riscv_vector::REDUCE_OP_FRM_DYN, operands, CONST0_RTX (mode)); @@ -850,7 +850,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED_AV, riscv_vector::REDUCE_OP_FRM_DYN, operands, operands[2]); DONE; @@ -878,7 +878,7 @@ else { rtx ops[] = {operands[0], operands[2], operands[3], operands[4]}; - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED_AV, riscv_vector::REDUCE_OP_M_FRM_DYN, ops, operands[1]); } @@ -1226,7 +1226,7 @@ { rtx ops[] = {operands[0], operands[2], operands[1], gen_int_mode (GET_MODE_NUNITS (mode), Pmode)}; - riscv_vector::expand_reduction (, + riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP_M, ops, CONST0_RTX (mode)); DONE; @@ -1281,7 +1281,7 @@ [(const_int 0)] { rtx ops[] = {operands[0], operands[3], operands[1], operands[2]}; - riscv_vector::expand_reduction (, + riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP_M, ops, CONST0_RTX (mode)); DONE; @@ -1317,7 +1317,7 @@ { rtx ops[] = {operands[0], operands[2], operands[1], gen_int_mode (GET_MODE_NUNITS (mode), Pmode)}; - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, riscv_vector::REDUCE_OP_M_FRM_DYN, ops, CONST0_RTX (mode)); DONE; @@ -1372,7 +1372,7 @@ [(const_int 0)] { rtx ops[] = {operands[0], operands[3], operands[1], operands[2]}; - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, riscv_vector::REDUCE_OP_M_FRM_DYN, ops, CONST0_RTX (mode)); DONE; diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md index 8d30ab22186..4432b7d1d05 100644 --- a/gcc/config/riscv/autovec.md +++ b/gcc/config/riscv/autovec.md @@ -2097,7 +2097,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (UNSPEC_REDUC_SUM, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (UNSPEC_REDUC_SUM_AV, riscv_vector::REDUCE_OP, operands, CONST0_RTX (mode)); DONE; } @@ -2110,7 +2110,7 @@ { int prec = GET_MODE_PRECISION (mode); rtx min = immed_wide_int_const (wi::min_value (prec, SIGNED), mode); - riscv_vector::expand_reduction (UNSPEC_REDUC_MAX, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (UNSPEC_REDUC_MAX_AV, riscv_vector::REDUCE_OP, operands, min); DONE; }) @@ -2120,7 +2120,7 @@ (match_operand:V_VLSI 1 "register_operand")] "TARGET_VECTOR" { - riscv_vector::expand_reduction (UNSPEC_REDUC_MAXU, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (UNSPEC_REDUC_MAXU_AV, riscv_vector::REDUCE_OP, operands, CONST0_RTX (mode)); DONE; }) @@ -2132,7 +2132,7 @@ { int prec = GET_MODE_PRECISION (mode); rtx max = immed_wide_int_const (wi::max_value (prec, SIGNED), mode); - riscv_vector::expand_reduction (UNSPEC_REDUC_MIN, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (UNSPEC_REDUC_MIN_AV, riscv_vector::REDUCE_OP, operands, max); DONE; }) @@ -2144,7 +2144,7 @@ { int prec = GET_MODE_PRECISION (mode); rtx max = immed_wide_int_const (wi::max_value (prec, UNSIGNED), mode); - riscv_vector
Re: [PATCH] libcpp: Fix overly large buffer allocation
On 12/18/24 5:04 PM, Lewis Hyatt wrote: Hello- Happened to notice this minor issue that seems worth fixing. bootstrap + regtest on x86-64, is it OK please? Thanks! -Lewis -- >8 -- It seems that tokens_buff_new() has always been allocating the virtual location buffer 4 times larger than intended, and now that location_t is 64-bit, it is 8 times larger. Fixed. libcpp/ChangeLog: * macro.cc (tokens_buff_new): Fix length argument to XNEWVEC. OK jeff
Re: [PATCH] RISC-V:Fix th.vsetvli generates from vext_x_v with wrong operand
On 12/19/24 12:56 AM, Robin Dapp wrote: From: Yunze Zhu Fix a bug th.vsetvli generates from vext_x_v with an imm operand, which reports illegal operand. This patch fix this by replacing imm operand with reg operand in th.vsetvli. LGTM but you might want to add check force_vector_length_operand which likely needs similar handling. Note that I'm also working with Jinma on a potentially related issue. In addition to fixing some initial codegen issues, there's concerns that IRA/LRA is substituting the constant back into the vsetvl instructions which doesn't work for thead. Anyway I think Yunze's patch probably stands on its own and is a step in the right direction. I'm inclined to commit it independently of Jinma's efforts. jeff
[PATCH v3] RISC-V: Fix code gen for reduction with length 0 [PR118182]
`.MASK_LEN_FOLD_LEFT_PLUS`(or `mask_len_fold_left_plus_m`) is expecting the return value will be the start value even if the length is 0. However current code gen in RISC-V backend is not meet that semantic, it will result a random garbage value if length is 0. Let example by current code gen for MASK_LEN_FOLD_LEFT_PLUS with f64: # _148 = .MASK_LEN_FOLD_LEFT_PLUS (stmp__148.33_134, vect__70.32_138, { -1, ... }, loop_len_161, 0); vsetvli zero,a5,e64,m1,ta,ma vfmv.s.fv2,fa5 # insn 1 vfredosum.vsv1,v1,v2 # insn 2 vfmv.f.sfa5,v1 # insn 3 insn 1: - vfmv.s.f won't do anything if VL=0, which means v2 will contain garbage value. insn 2: - vfredosum.vs won't do anything if VL=0, and keep vd unchanged even TA. (v-spec say: `If vl=0, no operation is performed and the destination register is not updated.`) insn 3: - vfmv.f.s will move the value from v1 even VL=0, so this is safe. So how we fix that? we need two fix for that: 1. insn 1: need always execute with VL=1, so that we can guarantee it will always work as expect. 2. insn 2: Add new pattern to force `vd` use same reg as `vs1` (start value) for all reduction patterns, then we can guarantee vd[0] will contain the start value when vl=0 For 1, it's just a simple change to riscv_vector::expand_reduction, but for 2, we have to add _AV variant reduction to force `vd` use same reg as `vs1` (start value). gcc/ChangeLog: * config/riscv/autovec-opt.md (*widen_reduc_plus_scal_): Use _AV variant of reduction expansion. (*widen_reduc_plus_scal_): Ditto. (*fold_left_widen_plus_): Ditto. (*mask_len_fold_left_widen_plus_): Ditto. (*cond_widen_reduc_plus_scal_): Ditto. (*cond_len_widen_reduc_plus_scal_): Ditto. (*cond_widen_reduc_plus_scal_): Ditto. * config/riscv/autovec.md (expand_reduction): Use _AV variant of reduction expansion, also always use VL=1 for setup start value. * config/riscv/riscv-v.cc: * config/riscv/vector-iterators.md (unspec): Add _AV variant of reduction. (ANY_REDUC_AV): New. (ANY_WREDUC_AV): Ditto. (ANY_FREDUC_AV): Ditto. (ANY_FREDUC_SUM_AV): Ditto. (ANY_FWREDUC_SUM_AV): Ditto. (reduc_op): Add _AV variant of reduction. (order) Ditto. * config/riscv/vector.md (@pred_av_): New. gcc/testsuite/ChangeLog: * gfortran.target/riscv/rvv/pr118182.f: New. --- gcc/config/riscv/autovec-opt.md | 16 +-- gcc/config/riscv/autovec.md | 30 ++--- gcc/config/riscv/riscv-v.cc | 4 +- gcc/config/riscv/vector-iterators.md | 44 +++ gcc/config/riscv/vector.md| 123 ++ .../gfortran.target/riscv/rvv/pr118182.f | 63 + 6 files changed, 255 insertions(+), 25 deletions(-) create mode 100644 gcc/testsuite/gfortran.target/riscv/rvv/pr118182.f diff --git a/gcc/config/riscv/autovec-opt.md b/gcc/config/riscv/autovec-opt.md index 4b33a145c17..45e3be4237b 100644 --- a/gcc/config/riscv/autovec-opt.md +++ b/gcc/config/riscv/autovec-opt.md @@ -810,7 +810,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP, + riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP, operands, CONST0_RTX (mode)); DONE; @@ -829,7 +829,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, riscv_vector::REDUCE_OP_FRM_DYN, operands, CONST0_RTX (mode)); @@ -850,7 +850,7 @@ "&& 1" [(const_int 0)] { - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED_AV, riscv_vector::REDUCE_OP_FRM_DYN, operands, operands[2]); DONE; @@ -878,7 +878,7 @@ else { rtx ops[] = {operands[0], operands[2], operands[3], operands[4]}; - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED, + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED_AV, riscv_vector::REDUCE_OP_M_FRM_DYN, ops, operands[1]); } @@ -1226,7 +1226,7 @@ { rtx ops[] = {operands[0], operands[2], operands[1], gen_int_mode (GET_MODE_NUNITS (mode), Pmode)}; - riscv_vector::expand_reduction (, + riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP_M, ops, CONST0_RTX (mode)); DONE; @@ -1281,7 +1281,7 @@ [(const_int 0)] { rtx ops[] = {operands[0], operands[3], operands[1], operands[2]
Re: [PATCH v2] RISC-V: Fix code gen for reduction with length 0 [PR118182]
sorry for my stupid mistake, forgot v2, it's just same as v1 and see v3 On Mon, Dec 23, 2024 at 11:15 PM Kito Cheng wrote: > > --- > gcc/config/riscv/autovec-opt.md | 16 ++-- > gcc/config/riscv/autovec.md | 30 +++ > gcc/config/riscv/riscv-v.cc | 4 +- > gcc/config/riscv/vector-iterators.md | 46 +++ > gcc/config/riscv/vector.md | 118 +++ > 5 files changed, 189 insertions(+), 25 deletions(-) > > diff --git a/gcc/config/riscv/autovec-opt.md b/gcc/config/riscv/autovec-opt.md > index 645dc53d868..4704bf342e3 100644 > --- a/gcc/config/riscv/autovec-opt.md > +++ b/gcc/config/riscv/autovec-opt.md > @@ -810,7 +810,7 @@ >"&& 1" >[(const_int 0)] > { > - riscv_vector::expand_reduction (, riscv_vector::REDUCE_OP, > + riscv_vector::expand_reduction (, > riscv_vector::REDUCE_OP, >operands, >CONST0_RTX (mode)); >DONE; > @@ -829,7 +829,7 @@ >"&& 1" >[(const_int 0)] > { > - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, > + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, >riscv_vector::REDUCE_OP_FRM_DYN, >operands, >CONST0_RTX (mode)); > @@ -850,7 +850,7 @@ >"&& 1" >[(const_int 0)] > { > - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED, > + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED_AV, >riscv_vector::REDUCE_OP_FRM_DYN, >operands, operands[2]); >DONE; > @@ -878,7 +878,7 @@ >else > { >rtx ops[] = {operands[0], operands[2], operands[3], operands[4]}; > - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED, > + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_ORDERED_AV, >riscv_vector::REDUCE_OP_M_FRM_DYN, >ops, operands[1]); > } > @@ -1226,7 +1226,7 @@ > { >rtx ops[] = {operands[0], operands[2], operands[1], > gen_int_mode (GET_MODE_NUNITS (mode), Pmode)}; > - riscv_vector::expand_reduction (, > + riscv_vector::expand_reduction (, >riscv_vector::REDUCE_OP_M, >ops, CONST0_RTX > (mode)); >DONE; > @@ -1281,7 +1281,7 @@ >[(const_int 0)] > { >rtx ops[] = {operands[0], operands[3], operands[1], operands[2]}; > - riscv_vector::expand_reduction (, > + riscv_vector::expand_reduction (, >riscv_vector::REDUCE_OP_M, >ops, CONST0_RTX > (mode)); >DONE; > @@ -1317,7 +1317,7 @@ > { >rtx ops[] = {operands[0], operands[2], operands[1], > gen_int_mode (GET_MODE_NUNITS (mode), Pmode)}; > - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, > + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, >riscv_vector::REDUCE_OP_M_FRM_DYN, >ops, CONST0_RTX > (mode)); >DONE; > @@ -1372,7 +1372,7 @@ >[(const_int 0)] > { >rtx ops[] = {operands[0], operands[3], operands[1], operands[2]}; > - riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED, > + riscv_vector::expand_reduction (UNSPEC_WREDUC_SUM_UNORDERED_AV, >riscv_vector::REDUCE_OP_M_FRM_DYN, >ops, CONST0_RTX > (mode)); >DONE; > diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md > index 8d30ab22186..4432b7d1d05 100644 > --- a/gcc/config/riscv/autovec.md > +++ b/gcc/config/riscv/autovec.md > @@ -2097,7 +2097,7 @@ >"&& 1" >[(const_int 0)] > { > - riscv_vector::expand_reduction (UNSPEC_REDUC_SUM, riscv_vector::REDUCE_OP, > + riscv_vector::expand_reduction (UNSPEC_REDUC_SUM_AV, > riscv_vector::REDUCE_OP, >operands, CONST0_RTX (mode)); >DONE; > } > @@ -2110,7 +2110,7 @@ > { >int prec = GET_MODE_PRECISION (mode); >rtx min = immed_wide_int_const (wi::min_value (prec, SIGNED), mode); > - riscv_vector::expand_reduction (UNSPEC_REDUC_MAX, riscv_vector::REDUCE_OP, > + riscv_vector::expand_reduction (UNSPEC_REDUC_MAX_AV, > riscv_vector::REDUCE_OP, >operands, min); >DONE; > }) > @@ -2120,7 +2120,7 @@ > (match_operand:V_VLSI 1 "register_operand")] >"TARGET_VECTOR" > { > - riscv_vector::expand_reduction (UNSPEC_REDUC_MAXU, riscv_vector::REDUCE_OP, > + riscv_vector::expand_reduction (UNSPEC_REDUC_MAXU_AV, > riscv_vector::REDUCE_OP, >operands, CONST0_RTX (mode)); >DONE; > }) > @@ -2132,7 +2132,7 @@ > { >int prec = GET_MODE_PRECISION (mode); >rtx max = immed_wide_int_const (wi::max_value (prec, SIGNED)
Re: [r15-6415 Regression] FAIL: gfortran.dg/coarray_lib_comm_1.f90 -Os scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 on Li
Hi all, I did an ooppsie with the patch for 107635. Attached is a patch that fixes this. Essentially the regexp was to complicated for what was needed. So now we just count the number of occurrences of caf_get_by_ct, which has to be four. Regtests ok on x86_64-pc-linux-gnu. Ok for mainline? Regards, Andre On Mon, 23 Dec 2024 04:52:50 +0800 "haochen.jiang" wrote: > On Linux/x86_64, > > 586477d67bf2e320e8ec41f82b194259c1dcc43a is the first bad commit > commit 586477d67bf2e320e8ec41f82b194259c1dcc43a > Author: Andre Vehreschild > Date: Fri Dec 6 08:57:34 2024 +0100 > > Fortran: Replace getting of coarray data with accessor-based version. > [PR107635] > > caused > > FAIL: gfortran.dg/coarray_lib_comm_1.f90 -O0 scan-tree-dump-times > original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned > long\\) atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 -O1 > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 FAIL: > gfortran.dg/coarray_lib_comm_1.f90 -O2 scan-tree-dump-times original > "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) > atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 -O3 > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 FAIL: > gfortran.dg/coarray_lib_comm_1.f90 -O3 -g scan-tree-dump-times original > "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) > atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 -Os > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 > > with GCC configured with > > ../../gcc/configure > --prefix=/export/users/haochenj/src/gcc-bisect/master/master/r15-6415/usr > --enable-clocale=gnu --with-system-zlib --with-demangler-in-ld > --with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl > --enable-libmpx x86_64-linux --disable-bootstrap > > To reproduce: > > $ cd {build_dir}/gcc && make check > RUNTESTFLAGS="dg.exp=gfortran.dg/coarray_lib_comm_1.f90 > --target_board='unix{-m32}'" $ cd {build_dir}/gcc && make check > RUNTESTFLAGS="dg.exp=gfortran.dg/coarray_lib_comm_1.f90 > --target_board='unix{-m32\ -march=cascadelake}'" > > (Please do not reply to this email, for question about this report, contact > me at haochen dot jiang at intel.com.) (If you met problems with cascadelake > related, disabling AVX512F in command line might save that.) (However, please > make sure that there is no potential problems with AVX512.) -- Andre Vehreschild * Email: vehre ad gmx dot de From b64c97530fc46b9c750a035925e14c77b2574d65 Mon Sep 17 00:00:00 2001 From: Andre Vehreschild Date: Mon, 23 Dec 2024 15:01:30 +0100 Subject: [PATCH] Fortran: Fixup broken build on 32bit after r15-6415 [PR107635] gcc/testsuite/ChangeLog: PR fortran/107635 * gfortran.dg/coarray_lib_comm_1.f90: Use less complicated pattern, because all we need is the right count. --- gcc/testsuite/gfortran.dg/coarray_lib_comm_1.f90 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gfortran.dg/coarray_lib_comm_1.f90 b/gcc/testsuite/gfortran.dg/coarray_lib_comm_1.f90 index 68aa47ecd32..609f3c10cef 100644 --- a/gcc/testsuite/gfortran.dg/coarray_lib_comm_1.f90 +++ b/gcc/testsuite/gfortran.dg/coarray_lib_comm_1.f90 @@ -38,6 +38,6 @@ B(1:5) = B(3:7) if (any (A-B /= 0)) STOP 4 end -! { dg-final { scan-tree-dump-times "_gfortran_caf_get_by_ct \\\(caf_token.., 0B, 0B, 1, \\\(unsigned long\\\) atmp.\[0-9\]+.span" 4 "original" } } +! { dg-final { scan-tree-dump-times "_gfortran_caf_get_by_ct" 4 "original" } } ! { dg-final { scan-tree-dump-times "_gfortran_caf_sendget \\\(caf_token.., \\\(integer\\\(kind=\[48\]\\\)\\\) parm.\[0-9\]+.data - \\\(integer\\\(kind=\[48\]\\\)\\\) a, 1, &parm.\[0-9\]+, 0B, caf_token.., \\\(integer\\\(kind=\[48\]\\\)\\\) parm.\[0-9\]+.data - \\\(integer\\\(kind=\[48\]\\\)\\\) a, 1, &parm.\[0-9\]+, 0B, 4, 4, 1, 0B\\\);" 1 "original" } } -- 2.47.1
Re: [PATCH] simplify-rtx: Limit number of elts in when encoding.
> from output_constant_pool_2 and make it defer to native_encode_rtx > instead. That seems like the most direct way of avoiding mishaps. > > E.g. another way in which different routines could make different choices > is in whether, for SVE's VNx8BI (say), they fill the upper bit of each > 2-bit element with 0s or with a copy of the low bit. Both choices are > valid in principle, and sharing the same code between both routines > would make sure that they make the same choice. Thanks. I went with your approach in the attached patch. It was bootstrapped and regtested on x86 and power10, aarch64 is still running. Regtested on rv64gcv_zvl512b. Regards Robin [PATCH] varasm: Use native_encode_rtx for constant vectors. optimize_constant_pool hashes vector masks by native_encode_rtx and merges identically hashed values in the constant pool. Afterwards the optimized values are written in output_constant_pool_2. However, native_encode_rtx and output_constant_pool_2 disagree in their encoding of vector masks: native_encode_rtx does not pad with zeroes while output_constant_pool_2 implicitly does. In RVV's shuffle-evenodd-run.c there are two masks (a) "0101" for V4BI (b) "01010101" for V8BI and that have the same representation/encoding ("1010101") in native_encode_rtx. output_constant_pool_2 uses "101" for (a) and "1010101" for (b). Now, optimize_constant_pool might happen to merge both masks using (a) as representative. Then, output_constant_pool_2 will output "1010" which is only valid for the second mask as the implicit zero padding doesn't agree with (b). (b)'s "1010101" works for both masks as a V4BI load will ignore the last four padding bits. This patch makes output_constant_pool_2 use native_encode_rtx so both functions will agree on an encoding and output the correct constant. gcc/ChangeLog: * varasm.cc (output_constant_pool_2): Use native_encode_rtx for building the memory image of a const vector mask. --- gcc/varasm.cc | 32 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/gcc/varasm.cc b/gcc/varasm.cc index 0068ec2ce4d..584d1864077 100644 --- a/gcc/varasm.cc +++ b/gcc/varasm.cc @@ -4301,34 +4301,26 @@ output_constant_pool_2 (fixed_size_mode mode, rtx x, unsigned int align) { gcc_assert (GET_CODE (x) == CONST_VECTOR); - /* Pick the smallest integer mode that contains at least one - whole element. Often this is byte_mode and contains more - than one element. */ + auto_vec buffer; + buffer.truncate (0); + buffer.reserve (GET_MODE_SIZE (mode)); + + bool ok = native_encode_rtx (mode, x, buffer, 0, GET_MODE_SIZE (mode)); + gcc_assert (ok); + unsigned int nelts = GET_MODE_NUNITS (mode); unsigned int elt_bits = GET_MODE_PRECISION (mode) / nelts; unsigned int int_bits = MAX (elt_bits, BITS_PER_UNIT); scalar_int_mode int_mode = int_mode_for_size (int_bits, 0).require (); - unsigned int mask = GET_MODE_MASK (GET_MODE_INNER (mode)); - /* We allow GET_MODE_PRECISION (mode) <= GET_MODE_BITSIZE (mode) but - only properly handle cases where the difference is less than a - byte. */ - gcc_assert (GET_MODE_BITSIZE (mode) - GET_MODE_PRECISION (mode) < - BITS_PER_UNIT); - - /* Build the constant up one integer at a time. */ - unsigned int elts_per_int = int_bits / elt_bits; - for (unsigned int i = 0; i < nelts; i += elts_per_int) + for (unsigned i = 0; +i < GET_MODE_SIZE (mode) / GET_MODE_SIZE (int_mode); +i += GET_MODE_SIZE (int_mode)) { unsigned HOST_WIDE_INT value = 0; - unsigned int limit = MIN (nelts - i, elts_per_int); - for (unsigned int j = 0; j < limit; ++j) - { - auto elt = INTVAL (CONST_VECTOR_ELT (x, i + j)); - value |= (elt & mask) << (j * elt_bits); - } + memcpy (&value, buffer.address () + i, GET_MODE_SIZE (int_mode)); output_constant_pool_2 (int_mode, gen_int_mode (value, int_mode), - i != 0 ? MIN (align, int_bits) : align); + align); } break; } -- 2.47.1
Re: [PATCH] RISC-V: Fix code gen for reduction with length 0 [PR118182]
On Mon, Dec 23, 2024 at 10:35 PM Robin Dapp wrote: > > > +;; Integer Reduction (vred(sum|maxu|max|minu|min|and|or|xor).vs), but for > > auto vectorizer > > +(define_insn "@pred_av_" > > + [(set (match_operand: 0 "register_operand" "=vr") > > + (unspec: > > + [(unspec: > > + [(match_operand: 1 "vector_mask_operand" "vmWc1") > > + (match_operand 5 "vector_length_operand" " rK") > > + (match_operand 6 "const_int_operand" "i") > > + (match_operand 7 "const_int_operand" "i") > > + (reg:SI VL_REGNUM) > > + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE) > > +(unspec: [ > > + (match_operand:V_VLSI3 "register_operand" " > > vr") > > + (match_operand: 4 "register_operand" " > > vr") > > + ] ANY_REDUC_AV) > > +(match_operand: 2 "vector_merge_operand" " vu")] > > UNSPEC_REDUC))] > > + "TARGET_VECTOR" > > + "v.vs\t%0,%3,%4%p1" > > + [(set_attr "type" "vired") > > + (set_attr "mode" "")]) > > Shouldn't operand 4 have a "0" constraint here and in the following two > changed > patterns? Or am I missing something? Good catch, apparently I missed that and spec 2006 didn't catch that as well...:P Will update at V2, > I'm not yet fully convinced we should go with this approach over the one with > the additional vsetvl (that might get merged with a preceding vsetvl and I guess this might be hard to merge unless the preceding vsetvl come with a constant VL. And here is LLVM's one: https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/RISCV/rvv/vreductions-int-vp.ll I am not trying to say it's the best or only way to resolve...but that's the only way I found...so brainstorming start! :P > therefore free). > > -- > Regards > Robin >
Re: [Patch, fortran] PR116254/118059 -[15 Regression] Bugs triggered by class_transformational_1/2.f90
Hi Paul, please run the style-checker on the patch before pushing. Furthermore, shouldn't that +changed, the dtype updating and the correct rank used. */ rather be +changed, the dtype updated and the correct rank used. */ ^^ Besides that the patch looks good to me. Ok for mainline. The best seasons greeting to you and everyone else, too. Thanks for the patch, Andre On Mon, 23 Dec 2024 07:50:18 + Paul Richard Thomas wrote: > Hi All, > > These bugs were tricky to find because neither were detected by regression > testing on my platform. > > PR116254: This bug was sporadic even where the regression was detected. > Richard Sandiford found "Conditional jump or move depends on uninitialised > value" errors in the library, triggered by the test for for spread in > class_transformational_2.f90. Mea culpa, I had failed to notice this. > It turned out that, alone of the rank changing intrinsic functions, spread > was going through the wrong code path. This is fixed by the > explicit condition in the patch. > > Richard also noted that spread results are being copied from uninitialised > memory. This was not quite correct because the library function was doing > the job on detection of the NULL descriptor data field. Nevertheless, I > have refactored slightly to ensure that the temporary descriptor has its > dtype field set correctly before the class data is pointed at it. Valgrind > shows both class_transformational tests to now be as clean as a whistle. > > PR118059: The reporter found, with an ubsan instrumented gcc, that > class_transformational_1.f90 was generating an invalid value for type > 'expr_t' in gcc/fortran/trans-expr.cc. The offending statement is; B = > reshape (chr, [5, 1, 2]), where 'B' is unlimited polymorphic. This requires > a trivial fix since the assignment of a character array valued function to > an unlimited polymorphic variable unconditionally requires a temporary. > > Regression tests without errors - OK for mainline? > > Seasons greetings to one and all. > > Paul -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [PATCH] libcc1: Fix tags generation target
On 12/23/24 6:11 AM, Simon Martin wrote: 'make tags' currently fails for libcc1 with this: *** No rule to make target `marshall-c.hh', needed by `tags-am'. Stop. The problem is that while marshall-c.hh has been removed via r12-454-g25d1a6ecdc443f, it's still part of the libcc1_la_SOURCES variable, hence the 'tags' target has a dependency on it. This patch simply removes the marshall_c_source variable, that should be empty. Successfully tested with 'make tags' at the root of the build directory. libcc1/ChangeLog: * Makefile.am: Remove reference to deleted marshall-c.h. * Makefile.in: Regenerate. OK jeff
Re: [r15-6415 Regression] FAIL: gfortran.dg/coarray_lib_comm_1.f90 -Os scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 on Linu
Hi Paul, thanks for the quick review. Pushed as gcc-15-6425-gdae506f73bd Thanks again, Andre On Mon, 23 Dec 2024 15:13:50 + Paul Richard Thomas wrote: > Hi Andre, > > It looks good to me. > > Thanks > > Paul > > > On Mon, 23 Dec 2024 at 14:58, Andre Vehreschild wrote: > > > Hi all, > > > > I did an ooppsie with the patch for 107635. Attached is a patch that fixes > > this. Essentially the regexp was to complicated for what was needed. So > > now we > > just count the number of occurrences of caf_get_by_ct, which has to be > > four. > > > > Regtests ok on x86_64-pc-linux-gnu. Ok for mainline? > > > > Regards, > > Andre > > > > On Mon, 23 Dec 2024 04:52:50 +0800 > > "haochen.jiang" wrote: > > > > > On Linux/x86_64, > > > > > > 586477d67bf2e320e8ec41f82b194259c1dcc43a is the first bad commit > > > commit 586477d67bf2e320e8ec41f82b194259c1dcc43a > > > Author: Andre Vehreschild > > > Date: Fri Dec 6 08:57:34 2024 +0100 > > > > > > Fortran: Replace getting of coarray data with accessor-based version. > > > [PR107635] > > > > > > caused > > > > > > FAIL: gfortran.dg/coarray_lib_comm_1.f90 -O0 scan-tree-dump-times > > > original "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned > > > long\\) atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 > > -O1 > > > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., > > 0B, > > > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 FAIL: > > > gfortran.dg/coarray_lib_comm_1.f90 -O2 scan-tree-dump-times original > > > "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) > > > atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 -O3 > > > -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer > > -finline-functions > > > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., > > 0B, > > > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 FAIL: > > > gfortran.dg/coarray_lib_comm_1.f90 -O3 -g scan-tree-dump-times > > original > > > "_gfortran_caf_get_by_ct \\(caf_token.., 0B, 0B, 1, \\(unsigned long\\) > > > atmp.[0-9]+.span" 4 FAIL: gfortran.dg/coarray_lib_comm_1.f90 -Os > > > scan-tree-dump-times original "_gfortran_caf_get_by_ct \\(caf_token.., > > 0B, > > > 0B, 1, \\(unsigned long\\) atmp.[0-9]+.span" 4 > > > > > > with GCC configured with > > > > > > ../../gcc/configure > > > --prefix=/export/users/haochenj/src/gcc-bisect/master/master/r15-6415/usr > > > --enable-clocale=gnu --with-system-zlib --with-demangler-in-ld > > > --with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet > > --without-isl > > > --enable-libmpx x86_64-linux --disable-bootstrap > > > > > > To reproduce: > > > > > > $ cd {build_dir}/gcc && make check > > > RUNTESTFLAGS="dg.exp=gfortran.dg/coarray_lib_comm_1.f90 > > > --target_board='unix{-m32}'" $ cd {build_dir}/gcc && make check > > > RUNTESTFLAGS="dg.exp=gfortran.dg/coarray_lib_comm_1.f90 > > > --target_board='unix{-m32\ -march=cascadelake}'" > > > > > > (Please do not reply to this email, for question about this report, > > contact > > > me at haochen dot jiang at intel.com.) (If you met problems with > > cascadelake > > > related, disabling AVX512F in command line might save that.) (However, > > please > > > make sure that there is no potential problems with AVX512.) > > > > > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > -- Andre Vehreschild * Email: vehre ad gmx dot de
Re: [PATCH v3] libstdc++: fix a dangling reference crash in ranges::is_permutation
On Sat, 21 Dec 2024, Giuseppe D'Angelo wrote: > Hello, > > On 20/12/2024 22:20, Patrick Palka wrote: > > > > The attached patch fixes it. I've tested on Linux x86-64. Adding a > > > > negative test for this is somehow challenging (how do you test you're > > > > not using a dangling reference?), but running the modified test under > > > > ASAN shows the fix in place. > > > > I'd expect a constexpr version of the test to reliably fail as soon as > > it encounters the UB. > > Good idea! added. > > > > > > > > > > > > Do you need me to create a report on bugzilla and cross-reference it > > > > from the patch? > > > > That'd be good since we'll probably want to backport the fix to the > > release branches and the PR could reflect that. > > Done, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118160 > > > > Makes sense, but it might be preferable for sake of QoI to fix this > > without introducing extra dereferences or projection applications > > if possible. > > > > auto&& is supposed to perform lifetime extension, but that only happens > > for an outermost temporary and not any temporaries within a > > subexpression IIUC. So how about if we use a second auto&& for the > > *__scan subexpression so that lifetime extension occurs there? > > > > > > > > libstdc++-v3/ChangeLog: > > > > > > * include/bits/ranges_algo.h (__is_permutation_fn): Do not cache > > > the projection result in a local variable, as that may create > > > dangling references. > > > * testsuite/25_algorithms/is_permutation/constrained.cc: Add a > > > test with a range returning prvalues. > > > > > > Signed-off-by: Giuseppe D'Angelo > > > --- > > > libstdc++-v3/include/bits/ranges_algo.h | 3 +-- > > > .../25_algorithms/is_permutation/constrained.cc | 11 +++ > > > 2 files changed, 12 insertions(+), 2 deletions(-) > > > > > > diff --git a/libstdc++-v3/include/bits/ranges_algo.h > > > b/libstdc++-v3/include/bits/ranges_algo.h > > > index 772bf4dd997..4d3c4325e2c 100644 > > > --- a/libstdc++-v3/include/bits/ranges_algo.h > > > +++ b/libstdc++-v3/include/bits/ranges_algo.h > > > @@ -567,9 +567,8 @@ namespace ranges > > > for (auto __scan = __first1; __scan != __last1; ++__scan) > > > { > > > - auto&& __proj_scan = std::__invoke(__proj1, *__scan); > > > auto __comp_scan = [&] (_Tp&& __arg) -> bool > > > { > > > - return std::__invoke(__pred, __proj_scan, > > > > If we go with the second auto&& approach then we should perfect forward > > __proj_scan here as you alluded to. That might seem unsafe at first > > glance (if say the project returns an rvalue) but since the predicate is > > regular_invocable it mustn't modify its arguments IIUC. > > Just to reiterate: if the projection returns an rvalue, it would be wrong for > the predicate to actually move from it, as it would violate the > equality-preserving semantic requirements of regular_invocable? I'll add the > missing forward then. IIUC yes. I just opened https://gcc.gnu.org/PR118185 to track a similar missing forward in our ranges::clamp implementation. > > New patch attached. > > Thanks, > -- > Giuseppe D'Angelo > > > Subject: [PATCH] libstdc++: fix a dangling reference crash in > ranges::is_permutation [PR118160] > > The code was caching the result of `invoke(proj, *it)` in a local > `auto &&` variable. The problem is that this may create dangling > references, for instance in case `proj` is `std::identity` (the common > case) and `*it` produces a prvalue: lifetime extension does not > apply here due to the expressions involved. > > Instead, store (and lifetime-extend) the result of `*it` in a separate > variable, then project that variable. While at it, also forward the > result of the projection to the predicate, so that the predicate can > act on the proper value category. > > libstdc++-v3/ChangeLog: > > PR libstdc++/118160 > * include/bits/ranges_algo.h (__is_permutation_fn): Avoid a > dangling reference by storing the result of the iterator > dereference and the result of the projection in two distinct > variables, in order to lifetime-extend each one. > Forward the projected value to the predicate. > * testsuite/25_algorithms/is_permutation/constrained.cc: Add a > test with a range returning prvalues. Test it in a constexpr > context, in order to rely on the compiler to catch UB. > > Signed-off-by: Giuseppe D'Angelo > --- > libstdc++-v3/include/bits/ranges_algo.h | 7 +-- > .../25_algorithms/is_permutation/constrained.cc | 13 + > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/libstdc++-v3/include/bits/ranges_algo.h > b/libstdc++-v3/include/bits/ranges_algo.h > index 772bf4dd997..80d8123443f 100644 > --- a/libstdc++-v3/include/bits/ranges_algo.h > +++ b/libstdc++-v3/include/bits/ranges_algo.h > @@ -567,9 +567,12 @@ namespace ranges > > for (auto __s
Re: [PATCH] c: do not warn about truncating NUL char when initializing nonstring arrays [PR117178]
Hi, Kees, (Sorry for the late reply, I was on vacation last week). I read all the comments of PR117178 and also your patch. Yes, I think the behavior of this patch is reasonable. The updated diagnostic messages are more accurate and helpful. And the testing cases look good. I am wondering whether the documentation of the command -Wunterminated-string-initialization or the attribute “nonstring” should be updated too to reflect such change? thanks. Qing > On Dec 15, 2024, at 23:02, Kees Cook wrote: > > When initializing a nonstring char array when compiled with > -Wunterminated-string-initialization the warning trips even when > truncating the trailing NUL character from the string constant. Only > warn about this when running under -Wc++-compat since under C++ we should > not initialize nonstrings from C strings. > > PR c/117178 > > gcc/c/ChangeLog: > >* c-typeck.cc (digest_init): Check for nonstring attribute and > avoid warning when only the trailing NUL is truncated. >(build_c_cast): Update function call prototype. >(store_init_value): Ditto. >(output_init_element): Ditto. > > gcc/testsuite/ChangeLog: > >* gcc.dg/Wunterminated-string-initialization.c: Update for > new warning text and check for nonstring cases. >* gcc.dg/Wunterminated-string-initialization-c++.c: Duplicate > C test for -Wc++-compat. > --- > gcc/c/c-typeck.cc | 73 --- > .../Wunterminated-string-initialization-c++.c | 28 +++ > .../Wunterminated-string-initialization.c | 26 ++- > 3 files changed, 98 insertions(+), 29 deletions(-) > create mode 100644 > gcc/testsuite/gcc.dg/Wunterminated-string-initialization-c++.c > > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc > index 10b02da8752d..5cd0c07b87b1 100644 > --- a/gcc/c/c-typeck.cc > +++ b/gcc/c/c-typeck.cc > @@ -116,8 +116,8 @@ static void push_member_name (tree); > static int spelling_length (void); > static char *print_spelling (char *); > static void warning_init (location_t, int, const char *); > -static tree digest_init (location_t, tree, tree, tree, bool, bool, bool, > bool, > - bool, bool); > +static tree digest_init (location_t, tree, tree, tree, tree, bool, bool, > bool, > + bool, bool, bool); > static void output_init_element (location_t, tree, tree, bool, tree, tree, > bool, > bool, struct obstack *); > static void output_pending_init_elements (int, struct obstack *); > @@ -6731,7 +6731,7 @@ build_c_cast (location_t loc, tree type, tree expr) > t = build_constructor_single (type, field, t); > if (!maybe_const) >t = c_wrap_maybe_const (t, true); > - t = digest_init (loc, type, t, > + t = digest_init (loc, field, type, t, > NULL_TREE, false, false, false, true, false, false); > TREE_CONSTANT (t) = TREE_CONSTANT (value); > return t; > @@ -8646,8 +8646,8 @@ store_init_value (location_t init_loc, tree decl, tree > init, tree origtype) > } > bool constexpr_p = (VAR_P (decl) > && C_DECL_DECLARED_CONSTEXPR (decl)); > - value = digest_init (init_loc, type, init, origtype, npc, int_const_expr, > - arith_const_expr, true, > + value = digest_init (init_loc, decl, type, init, origtype, npc, > + int_const_expr, arith_const_expr, true, > TREE_STATIC (decl) || constexpr_p, constexpr_p); > > /* Store the expression if valid; else report error. */ > @@ -8996,8 +8996,8 @@ check_constexpr_init (location_t loc, tree type, tree > init, >on initializers for 'constexpr' objects apply. */ > > static tree > -digest_init (location_t init_loc, tree type, tree init, tree origtype, > - bool null_pointer_constant, bool int_const_expr, > +digest_init (location_t init_loc, tree field, tree type, tree init, > + tree origtype, bool null_pointer_constant, bool int_const_expr, > bool arith_const_expr, bool strict_string, > bool require_constant, bool require_constexpr) > { > @@ -9132,27 +9132,46 @@ digest_init (location_t init_loc, tree type, tree > init, tree origtype, > && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST) >{ > unsigned HOST_WIDE_INT len = TREE_STRING_LENGTH (inside_init); > - unsigned unit = TYPE_PRECISION (typ1) / BITS_PER_UNIT; > - > - /* Subtract the size of a single (possibly wide) character > - because it's ok to ignore the terminating null char > - that is counted in the length of the constant. */ > - if (compare_tree_int (TYPE_SIZE_UNIT (type), len - unit) < 0) > - pedwarn_init (init_loc, 0, > - ("initializer-string for array of %qT " > - "is too long"), typ1); > - else if (warn_unterminated_string_initialization > - && compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0) > - warning_at (init_loc, OPT_Wunterminated_string_initialization, > -("initializer-string for array of %qT " > - "is too long"), typ1); > + > if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0) > { > - unsigned HOST_WIDE_INT size > -= tree_to_uhwi (TYP
[PATCH] testsuite: libstdc++: Use effective-target libatomic
Ok for trunk and releases/gcc-14? -- Test assumes libatomic.a is always available, but for some embedded targets, there is no libatomic.a and the test thus fail. libstdc++/ChangeLog: * 29_atomics/atomic_float/compare_exchange_padding.cc: Use effective-target libatomic_available. Signed-off-by: Torbjörn SVENSSON --- .../29_atomics/atomic_float/compare_exchange_padding.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc index 49626ac6651..9395e3026a7 100644 --- a/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc +++ b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc @@ -1,5 +1,6 @@ // { dg-do run { target c++20 } } // { dg-options "-O0" } +// { dg-require-effective-target libatomic_available } // { dg-additional-options "[atomic_link_flags [get_multilibs]] -latomic" } #include -- 2.25.1
4th Ping: [Middle-end][PATCH v4 0/3][RFC]Provide more contexts for -Warray-bounds and -Wstringop-* warning messages
Hi, This is the 4th ping of the middle end review for the patch set. Really appreciate any comments and suggestions from Middle-end reviewer on this patch (the diagnostic part of the patch has been reviewed and approved already). As I know, Kees and Sam have been using this option for a while and both found very helpful. Could you please take a look and let me know any issue in the patch? Thanks a lot! The latest version of(4th version) is: https://gcc.gnu.org/pipermail/gcc-patches/2024-November/667613.html https://gcc.gnu.org/pipermail/gcc-patches/2024-November/667614.html https://gcc.gnu.org/pipermail/gcc-patches/2024-November/667615.html https://gcc.gnu.org/pipermail/gcc-patches/2024-November/667616.html Qing Begin forwarded message: From: Sam James Subject: Re: 3rd Ping: [Middle-end][PATCH v4 0/3][RFC]Provide more contexts for -Warray-bounds and -Wstringop-* warning messages Date: December 6, 2024 at 13:32:55 EST To: Qing Zhao , Jeff Law Cc: richard Biener , GCC Patches , kees Cook , Andrew Pinski , David Malcolm Qing Zhao writes: This is the 3rd ping of the Middle-end review for this patch. Jeff, would you be able to take a look? (In part because I know you've had a lot of comments and feedback on the middle-end warnings before). The diagnostics bits are OK'd already. I've been running this on distro builds for a few months now and had great results with it so far (including finding some real bugs in packages that I'd previously dismissed as probable-FPs). I can also chuck it in to our general testing builds if it'd help any. Thanks a lot! Qing On Nov 26, 2024, at 10:30, Qing Zhao wrote: Another ping on the Middle-end review of this patch. This patch has been waiting for the middle-end review for a long time. Please review it and provide any feedback, I believe that this should be a nice improvement to GCC diagnostic in general. Thanks. Qing On Nov 15, 2024, at 10:34, Qing Zhao wrote: Gentle ping on the middle-end review for this patch. There are two parts of this patch: 1. Diagnostic part (Part 2), which has been reviewed by David; 2. Middle end part (Part 1 and 3), mainly on the copy_history information collection during transformation. Thanks, Qing On Nov 5, 2024, at 11:31, Qing Zhao wrote: Hi, This is the 4th version of the patch for fixing PR109071. Compared to the 3nd version: https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666870.html https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666872.html https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666871.html The major improvements to this patch are: 1. Divide the patch into 3 parts: Part 1: Add new data structure move_history, record move_history during transformation; Part 2: In warning analysis, Use the new move_history to form a rich location with a sequence of events, to report more context info of the warnings. Part 3: Add debugging mechanism for move_history. 2. Major change to the above Part 2, completely rewritten based on David's new class lazy_diagnostic_path. 3. Fix all issues identied By Sam; A. fix PR117375 (Bug in tree-ssa-sink.cc); B. documentation clarification; C. Add all the duplicated PRs in the commit comments; 4. Bootstrap GCC with the new -fdiagnostics-details on by default (Init (1)). exposed some ICE similar as PR117375 in tree-ssa-sink.cc, fixed. bootstrapping and regression testing on both x86 and aarch64. Please let me know any comment and suggestion. Thanks. Qing Qing Zhao (3): Provide more contexts for -Warray-bounds, -Wstringop-* warning messages due to code movements from compiler transformation (Part 1) [PR109071,PR85788,PR88771,PR106762,PR108770,PR115274,PR117179] Provide more contexts for -Warray-bounds, -Wstringop-* warning messages due to code movements from compiler transformation (Part 2) [PR109071,PR85788,PR88771,PR106762,PR108770,PR115274,PR117179] Provide more contexts for -Warray-bounds, -Wstringop-* warning messages due to code movements from compiler transformation (Part 3) [PR109071,PR85788,PR88771,PR106762,PR108770,PR115274,PR117179]
Re: [PATCH] Fortran: fix NULL without MOLD argument to scalar DT pointer dummy [PR118179]
On 12/23/24 9:19 AM, Harald Anlauf wrote: Dear all, while preparing the testcase null_actual_7.f90 for commit r15-6408, I overlooked a corner case, leading to a regression (PR118179). The obvious solution is to extend the suppression of copying back the pointer also for NULL actual arguments to pointer dummies with no specified intent. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald OK, and thanks for the fix. Jerry
[PATCH] Fortran: fix NULL without MOLD argument to scalar DT pointer dummy [PR118179]
Dear all, while preparing the testcase null_actual_7.f90 for commit r15-6408, I overlooked a corner case, leading to a regression (PR118179). The obvious solution is to extend the suppression of copying back the pointer also for NULL actual arguments to pointer dummies with no specified intent. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 49754a5acc6ca4c0927d54c3c00cc3f93f1ce4dd Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 23 Dec 2024 17:56:46 +0100 Subject: [PATCH] Fortran: fix NULL without MOLD argument to scalar DT pointer dummy [PR118179] Commit r15-6408 overlooked the case of passing NULL without MOLD argument to a derived type pointer dummy argument without specified intent. Since it is prohibited to modify the dummy argument, we treat it as if intent(in) were specified and suppress copying back of the pointer address. PR fortran/118179 gcc/fortran/ChangeLog: * trans-expr.cc (conv_null_actual): Suppress copying back of pointer address for unspecified intent. gcc/testsuite/ChangeLog: * gfortran.dg/null_actual_7.f90: Extend testcase to also cover scalar variants with pointer or allocatable dummy with or without specified intent. --- gcc/fortran/trans-expr.cc | 3 +- gcc/testsuite/gfortran.dg/null_actual_7.f90 | 77 + 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 9aedecb9780..4b022989e6f 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6488,7 +6488,8 @@ conv_null_actual (gfc_se * parmse, gfc_expr * e, gfc_symbol * fsym) int dummy_rank; tree tmp = parmse->expr; - if (fsym->attr.allocatable && fsym->attr.intent == INTENT_UNKNOWN) + if ((fsym->attr.allocatable || fsym->attr.pointer) + && fsym->attr.intent == INTENT_UNKNOWN) fsym->attr.intent = INTENT_IN; tmp = gfc_conv_scalar_to_descriptor (parmse, tmp, fsym->attr); dummy_rank = fsym->as ? fsym->as->rank : 0; diff --git a/gcc/testsuite/gfortran.dg/null_actual_7.f90 b/gcc/testsuite/gfortran.dg/null_actual_7.f90 index ba3cd10f21b..8891a3620ce 100644 --- a/gcc/testsuite/gfortran.dg/null_actual_7.f90 +++ b/gcc/testsuite/gfortran.dg/null_actual_7.f90 @@ -10,6 +10,8 @@ program null_actual end type t type(t), pointer :: p2(:,:) => NULL() type(t), allocatable :: a2(:,:) + type(t), pointer :: p0 => NULL () + type(t), allocatable :: a0 ! Basic tests passing unallocated allocatable / disassociated pointer stop_base = 0 @@ -27,6 +29,16 @@ program null_actual call chk2_t_p (p2) call opt2_t_a (a2) call opt2_t_p (p2) + ! ... to rank-0 dummy: + stop_base = 60 + call chk0_t_a (a0) + call chk0_t_p (p0) + call opt0_t_a (a0) + call opt0_t_p (p0) + call chk0_t_a_i (a0) + call chk0_t_p_i (p0) + call opt0_t_a_i (a0) + call opt0_t_p_i (p0) ! Test NULL with MOLD argument stop_base = 20 @@ -43,6 +55,16 @@ program null_actual call opt2_t_a (null(a2)) call opt2_t_p (null(p2)) + stop_base = 80 + call chk0_t_a (null(a0)) + call chk0_t_p (null(p0)) + call opt0_t_a (null(a0)) + call opt0_t_p (null(p0)) + call chk0_t_a_i (null(a0)) + call chk0_t_p_i (null(p0)) + call opt0_t_a_i (null(a0)) + call opt0_t_p_i (null(p0)) + ! Test NULL without MOLD argument stop_base = 40 call chk2_t_a (null()) @@ -50,6 +72,16 @@ program null_actual call opt2_t_a (null()) call opt2_t_p (null()) + stop_base = 100 + call chk0_t_a (null()) + call chk0_t_p (null()) + call opt0_t_a (null()) + call opt0_t_p (null()) + call chk0_t_a_i (null()) + call chk0_t_p_i (null()) + call opt0_t_a_i (null()) + call opt0_t_p_i (null()) + contains ! Check assumed-rank dummy: subroutine chk_t_a (x) @@ -120,4 +152,49 @@ contains if (.not. present (x)) stop stop_base + 19 if (associated (x))stop stop_base + 20 end subroutine opt2_t_p + + ! Checks for rank-0 dummy: + subroutine chk0_t_p (x) +type(t), pointer :: x +if (associated (x)) stop stop_base + 1 + end subroutine chk0_t_p + + subroutine chk0_t_p_i (x) +type(t), pointer, intent(in) :: x +if (associated (x)) stop stop_base + 2 + end subroutine chk0_t_p_i + + subroutine opt0_t_p (x) +type(t), pointer, optional :: x +if (.not. present (x)) stop stop_base + 3 +if (associated (x))stop stop_base + 4 + end subroutine opt0_t_p + + subroutine opt0_t_p_i (x) +type(t), pointer, optional, intent(in) :: x +if (.not. present (x)) stop stop_base + 5 +if (associated (x))stop stop_base + 6 + end subroutine opt0_t_p_i + + subroutine chk0_t_a (x) +type(t), allocatable :: x +if (allocated (x)) stop stop_base + 7 + end subroutine chk0_t_a + + subroutine chk0_t_a_i (x) +type(t), allocatable, intent(in) :: x +if (allocated (x)) stop stop_base + 8 + end subroutine chk0_t_a_i + + subroutine opt0_t_a (x) +type(t), allocatable, optional :: x +if (.not. present (x)) stop stop_bas
Re: [PATCH] testsuite: generalized field-merge tests for <32-bit int [PR118025]
On Sun, Dec 22, 2024 at 08:59:00PM -0300, Alexandre Oliva wrote: > Hello, Dimitar, > > On Dec 22, 2024, Dimitar Dimitrov wrote: > > > On Sat, Dec 21, 2024 at 02:28:33AM -0300, Alexandre Oliva wrote: > >> On Dec 20, 2024, Jakub Jelinek wrote: > >> > >> > On Wed, Dec 18, 2024 at 12:59:11AM -0300, Alexandre Oliva wrote: > >> >> * gcc.dg/field-merge-16.c: New. > >> > >> > Note the test FAILs on i686-linux or on x86_64-linux with -m32. > >> > >> Also fixed herein. > > Since the second patch adjusted one of the tests submitted in the first > patch, I'm holding off from installing the second until the first one > (URL below) is (hopefully) approved. > https://gcc.gnu.org/pipermail/gcc-patches/2024-December/672161.html > > >> Regstrapped on x86_64-linux-gnu. I'd appreciate if someone who can test > >> AVR and PRU would confirm that it fixes all field-merge-* failures. Ok > >> to install? > > > All failures are now fixed for PRU: > > make check-gcc-c RUNTESTFLAGS="--target_board=pru-sim > > dg.exp=field-merge-*.c" > > # of expected passes36 > > Thanks! > > > But several failures remain for AVR: > > FAIL: gcc.dg/field-merge-1.c (test for excess errors) > > FAIL: gcc.dg/field-merge-1.c execution test > [...] > > > I've attached the test log for AVR. > > Thanks, that was useful, if a little embarrasing (for my not realizing > that narrower int types would require explicit truncation of wide > constants ;-) > > > The tests seem to rely on int being at least 32 bits > > That reliance was accidental, rather than essential, so adding explicit > casts should be enough to avoid warnings and runtime errors. > > > > Explicitly convert constants to the desired types, so as to not elicit > warnings about implicit truncations, nor execution errors, on targets > whose ints are narrower than 32 bits. > > Tested (on top of the previous patches) on x86_64-linux-gnu, and on > avr-none, inspecting gcc.log and disregarding linker errors for not > finding -lm and -lc. I'd appreciate confirmation about execution tests > on avr (thanks in advance). Ok to install? There are no failing tests now for AVR: make check-gcc-c RUNTESTFLAGS="--target_board=atmega128-sim dg.exp=field-merge-*.c" # of expected passes36 Thanks, Dimitar
Re: [PATCH] Fortran: fix NULL without MOLD argument to scalar DT pointer dummy [PR118179]
Am 23.12.24 um 18:51 schrieb Jerry D: On 12/23/24 9:19 AM, Harald Anlauf wrote: Dear all, while preparing the testcase null_actual_7.f90 for commit r15-6408, I overlooked a corner case, leading to a regression (PR118179). The obvious solution is to extend the suppression of copying back the pointer also for NULL actual arguments to pointer dummies with no specified intent. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald OK, and thanks for the fix. Thanks, Jerry, for the incredibly fast review! Pushed: r15-6426 I'll leave the PR open to see if Jürgen can confirm that I did not miss another corner case... Cheers, Harald Jerry
Re: [PATCH v4 6/7] OpenMP: Fortran front-end support for dispatch + adjust_args
Hi PA, (next try, for some reasons, my original email disappeared.) Paul-Antoine Arras wrote: Replying to your last two messages here and attaching revised patches. Regarding the C++ and ME patches: ==> 0003-C-fix.patch <== Subject: [PATCH 3/4] C++ fix ==> 0004-ME-fixes.patch <== Subject: [PATCH 4/4] ME fixes I think it is best to fold them into the Fortran patch; otherwise, they would clearly need a better subject line. And for both changes and in either case, both need a ChangeLog entry. Additionally, the middle-end patch does not apply as it doesn't honor my Dec 18 change to gimplify.cc. * * * ==> 0001-OpenMP-Fortran-front-end-support-for-dispatch-adjust.patch <== Subject: [PATCH 1/4] OpenMP: Fortran front-end support for dispatch + adjust_args The following two patches do not work (at least with some testsuite testing) as in gcc/testsuite/ neither omp_lib nor libgomp.{so,a} is available. For gcc/testsuite/gfortran.dg/gomp/adjust-args-10.f90, you can just remove the 'omp_lib'. And as gcc/testsuite/gfortran.dg/gomp/declare-variant-21.f90 contains ! { dg-do run } ... I'd suggest to move it to libgomp (including its aux-21.f90 file). For adjust-args-10.f90, I wonder whether it is sufficient as compile-time only or whether it makes more sense to have a "dg-do run" to check that type(C_ptr) value vs. not-value works. I think either is fine, but if it stays in gcc/, can you manually run it once to re-check that it works? (I think I did check it and it worked.) * * * Note that gcc/testsuite/gfortran.dg/gomp/adjust-args-9.f90 and ...-10.f90 are missing a ChangeLog entry. Likewise for dispatch-9a.f90. BTW: If you have applied (committed) the patch locally, run ./contrib/gcc-changelog/git_check_commit.py -v -p — the '-v' will output new files that have not been listed as warning and -p shows the patch log for checking it. Additionally, it has the usual "git push" checks of GCC. * * * Otherwise, LGTM. Thanks! [As gimplify.cc couldn't be applied, I have not played with the patch but I believe that it should be okay, based on past playing and looking at the patch.] Tobias PS: Besides fixing the minor issues above, I think you have a follow-up/cleanup patch available addressing some issues related to the C/C++ FE, including where the '#pragma' is handled etc. I am looking forward to that follow-up patch as well :-)
Re: [PATCH v4] libstdc++: fix a dangling reference crash in ranges::is_permutation
Hello, On 23/12/2024 17:55, Patrick Palka wrote: Just to reiterate: if the projection returns an rvalue, it would be wrong for the predicate to actually move from it, as it would violate the equality-preserving semantic requirements of regular_invocable? I'll add the missing forward then. IIUC yes. I just opened https://gcc.gnu.org/PR118185 to track a similar missing forward in our ranges::clamp implementation. Ok. I've added a mention to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100249 in the changelog, as it seems relevant. +static_assert(test04); Missing (), otherwise LGTM! D'oh. Fourth time the charm... -- Giuseppe D'Angelo From 5d5e7186b927ef85f9e360d3bf9d2a98b9133251 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Fri, 20 Dec 2024 12:55:01 +0100 Subject: [PATCH] libstdc++: fix a dangling reference crash in ranges::is_permutation [PR118160] The code was caching the result of `invoke(proj, *it)` in a local `auto &&` variable. The problem is that this may create dangling references, for instance in case `proj` is `std::identity` (the common case) and `*it` produces a prvalue: lifetime extension does not apply here due to the expressions involved. Instead, store (and lifetime-extend) the result of `*it` in a separate variable, then project that variable. While at it, also forward the result of the projection to the predicate, so that the predicate can act on the proper value category. libstdc++-v3/ChangeLog: PR libstdc++/118160 PR libstdc++/100249 * include/bits/ranges_algo.h (__is_permutation_fn): Avoid a dangling reference by storing the result of the iterator dereference and the result of the projection in two distinct variables, in order to lifetime-extend each one. Forward the projected value to the predicate. * testsuite/25_algorithms/is_permutation/constrained.cc: Add a test with a range returning prvalues. Test it in a constexpr context, in order to rely on the compiler to catch UB. Signed-off-by: Giuseppe D'Angelo --- libstdc++-v3/include/bits/ranges_algo.h | 7 +-- .../25_algorithms/is_permutation/constrained.cc | 13 + 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h index 772bf4dd997..80d8123443f 100644 --- a/libstdc++-v3/include/bits/ranges_algo.h +++ b/libstdc++-v3/include/bits/ranges_algo.h @@ -567,9 +567,12 @@ namespace ranges for (auto __scan = __first1; __scan != __last1; ++__scan) { - auto&& __proj_scan = std::__invoke(__proj1, *__scan); + auto&& __scan_deref = *__scan; + auto&& __proj_scan = + std::__invoke(__proj1, std::forward(__scan_deref)); auto __comp_scan = [&] (_Tp&& __arg) -> bool { - return std::__invoke(__pred, __proj_scan, + return std::__invoke(__pred, + std::forward(__proj_scan), std::forward<_Tp>(__arg)); }; if (__scan != ranges::find_if(__first1, __scan, diff --git a/libstdc++-v3/testsuite/25_algorithms/is_permutation/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/is_permutation/constrained.cc index 2fbebe37609..7266aff5b17 100644 --- a/libstdc++-v3/testsuite/25_algorithms/is_permutation/constrained.cc +++ b/libstdc++-v3/testsuite/25_algorithms/is_permutation/constrained.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -76,10 +77,22 @@ test03() while (std::next_permutation(std::begin(cx), std::end(cx))); } +constexpr +bool +test04() // PR118160, do not create dangling references +{ + int x[] = { 4, 3, 2, 1 }; + auto y = std::views::iota(1, 5); + return ranges::is_permutation(x, y) && ranges::is_permutation(y, x); +} + +static_assert(test04()); + int main() { test01(); test02(); test03(); + VERIFY( test04() ); } -- 2.34.1 smime.p7s Description: S/MIME Cryptographic Signature
[COMMITTED] libgfortran: Fix build for targets with int32_t=long int
Not many newlib targets (IIRC the only targets where int32_t is a typedef of long int) build libgfortran. Building and testing fortran testsuite is usually a cheap way to get additional coverage for a port, except that a couple of times a year, there are these silly type-related breakages. Maybe change to building libgfortran as C++? 1/2 :-) Committed as obvious after a build of libgfortran for cris-elf succeeded with this patch. -- >8 -- Without this, after r15-6415-g586477d67bf2e3, you'll see, for targets where int32_t is a typedef of long int (beware of artificially broken lines): /x/gcc/libgfortran/caf/single.c: In function '_gfortran_caf_get_by_ct': /x/gcc/libgfortran/caf/single.c:2943:56: error: passing argument 2 of '\ (accessor_hash_table + (sizetype)((unsigned int)getter_index * 12))->ac\ cessor' from incompatible pointer type [-Wincompatible-pointer-types] 2943 | accessor_hash_table[getter_index].accessor (dst_ptr, &free_bu\ ffer, src_ptr, |^~~~\ || |int * /x/gcc/libgfortran/caf/single.c:2943:56: note: expected 'int32_t *' {ak\ a 'long int *'} but argument is of type 'int *' libgfortran: * caf/single.c (_gfortran_caf_get_by_ct): Correct type of free_buffer to int32_t. --- libgfortran/caf/single.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libgfortran/caf/single.c b/libgfortran/caf/single.c index f5414ff1f7ef..23ad44e1c162 100644 --- a/libgfortran/caf/single.c +++ b/libgfortran/caf/single.c @@ -2927,7 +2927,7 @@ _gfortran_caf_get_by_ct ( { caf_single_token_t single_token = TOKEN (token); void *src_ptr = opt_src_desc ? (void *) opt_src_desc : single_token->memptr; - int free_buffer; + int32_t free_buffer; void *dst_ptr = opt_dst_desc ? (void *)opt_dst_desc : dst_data; void *old_dst_data_ptr = NULL; -- 2.30.2
[PATCH] testsuite: libitm: Remove build directory path from test names
Hello- This patch helps tools like contrib/compare_tests work out of the box for the libitm testsuite. Without this change, compare_tests complains if the test runs being compared were in different build directories. I just moved the -B argument from one place to another, so the command line executed by runtest is more or less the same as before. FWIW however, I was not able to confirm it entirely because on every platform I tried (x86-64-linux, sparc, aarch64, x86_64-apple-darwin24.1.0), the tests also work fine with the -B argument omitted entirely. I was not able to figure out under what circumstances the -B is needed, given that -L is already passed with the same location. I see that it was added in r14-4827, which was about fixing the tests on newer versions of darwin. Thanks! -Lewis -- >8 -- The libitm c++ tests pass an extra -B option argument to all dg-runtest invocations, where depends on the absolute path to the build directory, to help find libstdc++. This argument ends up as part of each test name in the test results summary, so an extra filtering step is often required to avoid spurious complaints from test result comparison tools like contrib/compare_tests, since it is common to compare test results from two different build directories. Fixed by applying the -B option in libitm_target_compile directly, so that dg-runtest doesn't see it and add it to the test name. libitm/ChangeLog: * testsuite/libitm.c++/c++.exp: Move -B argument for libstdc++ to... * testsuite/lib/libitm.exp (libitm_target_compile): ...here. --- libitm/testsuite/lib/libitm.exp | 1 + libitm/testsuite/libitm.c++/c++.exp | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libitm/testsuite/lib/libitm.exp b/libitm/testsuite/lib/libitm.exp index ac390d6d0dd..41acbfca58e 100644 --- a/libitm/testsuite/lib/libitm.exp +++ b/libitm/testsuite/lib/libitm.exp @@ -199,6 +199,7 @@ proc libitm_target_compile { source dest type options } { if { [info exists lang_test_file] } { if { $blddir != "" } { lappend options "ldflags=-L${blddir}/${lang_library_path}" + lappend options "additional_flags=-B${blddir}/${lang_library_path}" } lappend options "ldflags=${lang_link_flags}" } diff --git a/libitm/testsuite/libitm.c++/c++.exp b/libitm/testsuite/libitm.c++/c++.exp index ab278f2cb33..906f51aac66 100644 --- a/libitm/testsuite/libitm.c++/c++.exp +++ b/libitm/testsuite/libitm.c++/c++.exp @@ -56,10 +56,8 @@ if { $lang_test_file_found } { # Gather a list of all tests. set tests [lsort [glob -nocomplain $srcdir/$subdir/*.C]] -set stdcxxadder "" if { $blddir != "" } { set ld_library_path "$always_ld_library_path:${blddir}/${lang_library_path}" - set stdcxxadder "-B ${blddir}/${lang_library_path}" } else { set ld_library_path "$always_ld_library_path" } @@ -74,7 +72,7 @@ if { $lang_test_file_found } { } # Main loop. -dg-runtest $tests $stdcxxadder $libstdcxx_includes +dg-runtest $tests "" $libstdcxx_includes } # All done.
RE: [PATCH] COBOL 3/8 gen: GENERIC interface
Richard, a bunch of things you address are in my bailwick. When Jim and I set out to create a COBOL front end, I knew *NOTHING* about, well, anything vis-à-vis GCC. I barely knew how it worked. Some things I had to figure out even before I knew how to figure anything outl notably, creating functions. Creating a function was practically the first thing I had to do, and I spent a bunch of time trapping through GCC compiling simple functions to devine how it might be done. By definition, I simply did not know what I was doing, and learned as I went. So, if there are peculiarities, that's probably why. And I am going to eagerly go through this message, and others, to apply the knowledge you are conveying; I am in a far better position to understand what you are saying than I would have been three years ago. > -Original Message- > From: Richard Biener > Sent: Sunday, December 22, 2024 07:18 > To: jklow...@symas.com > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] COBOL 3/8 gen: GENERIC interface > > > (sorry for breaking threading, but quoting the whole mail made my MUA > unbearably slow) > > From 64bcb34e12371f61a8958645e1668e0ac2704391gen.patch 4 Oct 2024 12:01:22 > -0400 > From: "James K. Lowden" > Date: Thu 12 Dec 2024 06:28:07 PM EST > Subject: [PATCH] Add 'cobol' to 4 files > > gcc/cobol/ChangeLog > * genapi.cc: New file. > * gengen.cc: New file. > * genmath.cc: New file. > * genutil.cc: New file. > > > I'm skimming over this part of the frontend, taking notes on general > things I notice, partly quoting parts of the patch > > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "config.h" > +#include "system.h" > > all system headers need to be included by system.h, C++ includes should be > included by defining INCLUDE_UNORDERED_MAP and INCLUDE_UNORDERED_SET, etc. > - includes definitely have to come after including config.h I now see that many of the front ends seem to have their own lang-system.h that wraps the gcc/system.h. Under the assumption that the recently-added GO and RUST are doing that in an acceptable fashion, I will model that behavior. > > +//#define XXX do{gg_printf("LINE %d\n", build_int_cst_type(INT, > __LINE__), NULL_TREE);}while(0); > +#define XXX > ... > +bool bSHOW_PARSE = getenv("SHOW_PARSE"); > > there seems to be debugging wired in - at least the getenv call above > should probably be be behind a more global COBOL_DEBUG conditional? > There's a langhook for frontend initialization - that's a convenient place > to conditionally (re-)initialize bSHOW_PARSE (when you default it to > false). > > + // One strongly suspects, one does, that creating the constructor one > + // character at a time isn't the best possible way of accomplishing > + the // desired goal. But I lost interest in trying to find a better > way. > + TYPE_NAME(array_of_characters) = get_identifier("cblc_string"); tree > + constr = make_node(CONSTRUCTOR); > + TREE_TYPE(constr) = array_of_characters; > > indeed, you should be able to use build_string (strlen(var_contents)+1, > var_contents) and use that as DECL_INTIIAL (it builds a STRING_CST node). That worked, thank you. > > +tree > +gg_fprintf(tree fd, int nargs, const char *format_string, ...) > + { > ... > + tree stmt = build_call_array_loc (location_from_lineno(), > +INT, > +function, > +argc, > +args); > > instead of constructing args[] you should be able to directly call > build_call_valist () with the va_list. > In this case, I don't see how to use build_call_valist (). gg_fprintf() takes a format_string and 'nargs' following arguments. It then builds a call to sprintf (buffer, format_string, You seem to, in function_decl_from_name or gg_get_function_address, build > function decls for externs, mainly from the C library. Elsewhere we are > using what GCC calls "built-ins" for this, see gcc/builtins.def and > tree.h:builtin_decl_explicit. Unfortunately most builtins and the used > types are to be initialized by frontends, luckily some are from the > middle-end, see build_common_builtin_nodes - that includes BUILT_IN_MEMCPY > and friends. For optimization purposes (that GCC knows that "memcpy" is > memcpy) I suggest you see to properly declare > BUILT_IN_* declarations - the Fortran frontend might be a good recipie to > copy from (see fortran/f95-lang.cc around where it includes > mathbuiltins.def). I will learn about builtins. > > +if( !field->var_decl_node ) > + { > + fprintf( stderr, > +"%s (type: %s) improperly has a NULL var_decl_node\n", > +field->name, > +cbl_field_type_str(field->type)); > + fprintf( stderr, > +"Probable cause: it was ref