Re: [patch][i386] Adding pconfig, wbnoinvd and wbinvd intrinsics
On Wed, Mar 14, 2018 at 1:39 PM, Makhotina, Olga wrote: > Hi, > > I have made changes to this patch. > I attached a new version. > > 14.03. 2018 Olga Makhotina > > gcc/ > * config/i386/sgxintrin.h (_enclv_u32): New intrinsic. > (__enclv_bc, __enclv_cd, __enclv_generic): New definitions. > (ERDINFO, ETRACKC, ELDBC, ELDUC): New leaves. > > gcc/testsuite/ > * gcc.target/i386/sgx.c (_enclv_u32): Test new intrinsic. > > Is it ok for trunk? OK. Thanks, Uros. > Thanks, Olga. > > -Original Message- > From: Uros Bizjak [mailto:ubiz...@gmail.com] > Sent: Sunday, March 4, 2018 8:23 PM > To: Makhotina, Olga > Cc: gcc-patches@gcc.gnu.org; Kirill Yukhin > Subject: Re: [patch][i386] Adding pconfig, wbnoinvd and wbinvd intrinsics > > On Fri, Mar 2, 2018 at 3:15 PM, Makhotina, Olga > wrote: >> Hi, >> >> I have made changes to this patch. >> I attached a new version. >> >> 02.03.2018 Olga Makhotina >> >> gcc/ >> * common/config/i386/i386-common.c (OPTION_MASK_ISA_PCONFIG_SET, >> OPTION_MASK_ISA_PCONFIG_UNSET, OPTION_MASK_ISA_WBNOINVD_SET, >> OPTION_MASK_ISA_WBNOINVD_UNSET): New definitions. >> (ix86_handle_option): Handle -mpconfig and -mwbnoinvd. >> * config.gcc (pconfigintrin.h, wbnoinvdintrin.h) : Add headers. >> * config/i386/cpuid.h (bit_PCONFIG, bit_WBNOINVD): New. >> * config/i386/driver-i386.c (host_detect_local_cpu): Detect -mpconfig >> and -mwbnoinvd. >> * config/i386/i386-builtin.def (__builtin_ia32_wbnoinvd, >> __builtin_ia32_wbinvd): New builtins. >> (SPECIAL_ARGS2): New. >> * config/i386/i386-c.c (__WBNOINVD__, __PCONFIG__): New. >> (SPECIAL_ARGS2): New. >> * config/i386/i386.c (ix86_target_string): Add -mpconfig and >> -mwbnoinvd. >> (ix86_valid_target_attribute_inner_p): Ditto. >> (ix86_init_mmx_sse_builtins): Add special_args2. >> * config/i386/i386.h (TARGET_PCONFIG, TARGET_PCONFIG_P, >> TARGET_WBNOINVD, >> TARGET_WBNOINVD_P): New. >> * config/i386/i386.md (UNSPECV_WBINVD, UNSPECV_WBNOINVD): New. >> (define_insn "wbinvd", define_insn "wbnoinvd"): New. >> * config/i386/i386.opt: Add -mpconfig and -mwbnoinvd. >> * config/i386/immintrin.h (_wbinvd): New intrinsic. >> * config/i386/pconfigintrin.h: New file. >> * config/i386/wbnoinvdintrin.h: Ditto. >> * config/i386/x86intrin.h: Add headers pconfigintrin.h and >> wbnoinvdintrin.h. >> * doc/invoke.texi (-mpconfig, -mwbnoinvd): New. >> >> gcc/testsuite/ >> * g++.dg/other/i386-2.C: Add -mpconfig and -mwbnoinvd. >> * g++.dg/other/i386-3.C: Ditto. >> * gcc.target/i386/sse-12.c: Ditto. >> * gcc.target/i386/sse-13.c: Ditto. >> * gcc.target/i386/sse-14.c: Ditto. >> * gcc.target/i386/sse-23.c: Add pconfig and wbnoinvd. >> * gcc.target/i386/wbinvd-1.c: New test. >> * gcc.target/i386/wbnoinvd-1.c: Ditto. >> * gcc.target/i386/pconfig-1.c: Ditto. >> >> Is it ok for trunk? > > OK. > > Thanks, > Uros.
Re: [PATCH] Fix emit_conditional_move (PR target/84860)
On Wed, 14 Mar 2018, Jakub Jelinek wrote: > Hi! > > prepare_cmp_insn in some cases changes both the passed in comparison and the > comparison mode, e.g. by promoting the arguments from SFmode to DFmode etc. > > In emit_conditional_move we call this in a loop, for (pass = 0; pass < 2; > pass++) > and in each case construct comparison arguments as well as the comparison > passed to it from the original arguments (we have to after all, because when > not successful, we throw the whole insn sequence away), but use cmode which > the first iteration could have changed, on this testcase on powerpcspe with > -mcpu=8548 from SFmode to DFmode, so we ICE the second time, because the > arguments don't really match the comparison mode. > > Fixed by passing it an address of a copy of the cmode parameter, so that the > second pass starts with the original cmode that matches the arguments again. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Richard. > 2018-03-14 Jakub Jelinek > > PR target/84860 > * optabs.c (emit_conditional_move): Pass address of cmode's copy > rather than address of cmode as last argument to prepare_cmp_insn. > > * gcc.c-torture/compile/pr84860.c: New test. > > --- gcc/optabs.c.jj 2018-02-09 19:11:29.0 +0100 > +++ gcc/optabs.c 2018-03-14 09:22:31.707873477 +0100 > @@ -4345,9 +4345,10 @@ emit_conditional_move (rtx target, enum > save_pending_stack_adjust (&save); > last = get_last_insn (); > do_pending_stack_adjust (); > + machine_mode cmpmode = cmode; > prepare_cmp_insn (XEXP (comparison, 0), XEXP (comparison, 1), > GET_CODE (comparison), NULL_RTX, unsignedp, > - OPTAB_WIDEN, &comparison, &cmode); > + OPTAB_WIDEN, &comparison, &cmpmode); > if (comparison) > { > struct expand_operand ops[4]; > --- gcc/testsuite/gcc.c-torture/compile/pr84860.c.jj 2018-03-14 > 09:26:19.93506 +0100 > +++ gcc/testsuite/gcc.c-torture/compile/pr84860.c 2018-03-14 > 09:26:44.854884590 +0100 > @@ -0,0 +1,11 @@ > +/* PR target/84860 */ > + > +void > +foo (int x, int y) > +{ > + while (x < 1) > +{ > + x = y; > + y = ((float)1 / 0) ? 2 : 0; > +} > +} > > Jakub > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH] Add test-case (PR ipa/84805).
On 14 March 2018 at 21:12, Eric Botcazou wrote: >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Please make sure to test it on more platforms (see PR ipa/83983 for details). > Hi, These new tests do not work well on arm-none-eabi and aarch64-none-elf. (as opposed to arm*linux* and aarch64*linux*) On arm-none-eabi, I see: /aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/arm-none-eabi/bin/ld: /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/gcc/testsuite/g++1/../../crtbegin.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol' can not be used when making a shared object; recompile with -fPIC /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/gcc/testsuite/g++1/../../crtbegin.o: error adding symbols: Bad value collect2: error: ld returned 1 exit status compiler exited with status 1 FAIL: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_0.C line 106) FAIL: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_0.C line 142) FAIL: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_0.C line 147) FAIL: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_1.C line 6) FAIL: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_2.C line 18) FAIL: g++.dg/lto/pr84805 cp_lto_pr84805_0.o-cp_lto_pr84805_2.o link, -O2 -fPIC -shared -flto On aarch64-none-elf, I see: /aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/aarch64-none-elf/bin/ld: /aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/lib/gcc/aarch64-none-elf/8.0.1/../../../../aarch64-none-elf/lib/librdimon.a(rdimon-syscalls.o): relocation R_AARCH64_ADR_PREL_PG_HI21 against external symbol `_impure_ptr' can not be used when making a shared object; recompile with -fPIC PASS: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_0.C line 106) PASS: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_0.C line 142) PASS: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_0.C line 147) PASS: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_1.C line 6) PASS: g++.dg/lto/pr84805 (test for LTO warnings, pr84805_2.C line 18) FAIL: g++.dg/lto/pr84805 cp_lto_pr84805_0.o-cp_lto_pr84805_2.o link, -O2 -fPIC -shared -flto Christophe > -- > Eric Botcazou
Re: [C++ PATCH] Fix up -Wdeprecated (PR c++/84222)
On Wed, Mar 14, 2018 at 08:44:48PM -0400, Jason Merrill wrote: > > --- gcc/cp/decl.c.jj2018-03-14 09:44:55.744974946 +0100 > > +++ gcc/cp/decl.c 2018-03-14 12:18:08.094012453 +0100 > > @@ -10448,7 +10448,7 @@ grokdeclarator (const cp_declarator *dec > > suppress reports of deprecated items. */ > >if (type && TREE_DEPRECATED (type) > >&& deprecated_state != DEPRECATED_SUPPRESS) > > -warn_deprecated_use (type, NULL_TREE); > > +cp_warn_deprecated_use (type); > >if (type && TREE_CODE (type) == TYPE_DECL) > > { > >typedef_decl = type; > > @@ -10456,7 +10456,7 @@ grokdeclarator (const cp_declarator *dec > >if (TREE_DEPRECATED (type) > > && DECL_ARTIFICIAL (typedef_decl) > > && deprecated_state != DEPRECATED_SUPPRESS) > > - warn_deprecated_use (type, NULL_TREE); > > + cp_warn_deprecated_use (type); > > } > >/* No type at all: default to `int', and set DEFAULTED_INT > > because it was not a user-defined typedef. */ > > @@ -11271,8 +11271,18 @@ grokdeclarator (const cp_declarator *dec > > explicitp = 2; > > } > > > > - arg_types = grokparms (declarator->u.function.parameters, > > - &parms); > > + tree pushed_scope = NULL_TREE; > > + if (funcdecl_p > > + && decl_context != FIELD > > + && inner_declarator->u.id.qualifying_scope > > + && CLASS_TYPE_P (inner_declarator->u.id.qualifying_scope)) > > + pushed_scope > > + = push_scope (inner_declarator->u.id.qualifying_scope); > > Can't we use ctype here? Inside of classes ctype is non-NULL, but we don't need to push anything, current_class_type is already the class we care about. That's the tree ctype = current_class_type; on line 10130. Then we have this for (id_declarator = declarator; id_declarator; id_declarator = id_declarator->declarator) loop, where for cdk_id at line 10242 it tweaks ctype: else if (TYPE_P (qualifying_scope)) { ctype = qualifying_scope; if (!MAYBE_CLASS_TYPE_P (ctype)) { error ("%q#T is not a class or a namespace", ctype); ctype = NULL_TREE; } and indeed for the cases I care about (out of class method definitions) ctype is set to non-NULL here. But then at line 10542 it is cleared again: ctype = NULL_TREE; and at 11176: if (ctype == NULL_TREE && decl_context == FIELD && funcdecl_p && friendp == 0) ctype = current_class_type; set only for selected in-class definitions, and then tested and used a couple of times. And that is the state we call grokparms with. Only later at line 11529 it is set again: if (declarator && declarator->kind == cdk_id && declarator->u.id.qualifying_scope && MAYBE_CLASS_TYPE_P (declarator->u.id.qualifying_scope)) { ctype = declarator->u.id.qualifying_scope; ctype = TYPE_MAIN_VARIANT (ctype); So, if I were to use some variable without really changing the behavior of the grokdeclarator massively, it would need to be a copy of ctype saved into another variable (how it should be named?) above line 10542. Given the TYPE_MAIN_VARIANT, I guess we should be using TYPE_MAIN_VARIANT somewhere too. Jakub
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
On Wed, Mar 14, 2018 at 08:55:47PM -0400, Jason Merrill wrote: > > @@ -3192,16 +3198,70 @@ replace_placeholders (tree exp, tree obj > > return exp; > > > >tree *tp = &exp; > > - hash_set pset; > > - replace_placeholders_t data = { obj, false, &pset }; > > + /* Use exp instead of *(type *)&exp. */ > > + while (TREE_CODE (exp) == INDIRECT_REF) > > +{ > > + tree t = TREE_OPERAND (exp, 0); > > + STRIP_NOPS (t); > > + if (TREE_CODE (t) == ADDR_EXPR > > + && (t = TREE_OPERAND (t, 0)) > > + && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (exp), > > + TREE_TYPE (t))) > > + exp = t; > > + else > > + break; > > +} > > What's the motivation for this? g++.dg/cpp0x/nsdmi13.C ICEs without that, we have there: a = A({}); and build_over_call does: 8163 else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (as_base))) 8164{ 8165 arg = cp_build_fold_indirect_ref (arg); 8166 val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); 8167 /* Handle NSDMI that refer to the object being initialized. */ 8168 replace_placeholders (arg, to); 8169} where arg is after the cp_build_fold_indirect_ref *(struct A &) &TARGET_EXPR }> This is in MODIFY_EXPR rather than INIT_EXPR and the gimplifier through gimple_fold_indirect_ref_rhs folds the *(struct A &) & away and so there is no further temporary and thus cp_gimplify_init_expr isn't called to replace_placeholders, so if we don't replace it here (with to being the a VAR_DECL), we don't replace it ever. > I suspect that it should only apply if REFERENCE_REF_P. Incremental --- gcc/cp/tree.c~ 2018-03-15 09:13:38.0 +0100 +++ gcc/cp/tree.c 2018-03-15 09:15:43.029087739 +0100 @@ -3199,7 +3199,7 @@ replace_placeholders (tree exp, tree obj tree *tp = &exp; /* Use exp instead of *(type *)&exp. */ - while (TREE_CODE (exp) == INDIRECT_REF) + while (REFERENCE_REF_P (exp)) { tree t = TREE_OPERAND (exp, 0); STRIP_NOPS (t); works too. I think the gimplifier would gimple_fold_indirect_ref_rhs away even *(A *) &TARGET_EXPR, but maybe that just can't happen in C++. The point in the replace_placeholders data.exp computation is to differentiate between binding a CONSTRUCTOR to a particular object and e.g. the C c = bar (X {1}); case from pr79937-1.C where it is just an expression that contains a CONSTRUCTOR in it. In the former case, e.g. the nsdmi13.C, we want to replace placeholders in it, while in the latter we don't; in both cases there is just one CONSTRUCTOR, so say ignoring the new flag on outermost CONSTRUCTOR we process doesn't work. Jakub
Re: RFA (make_dispatcher_decl): PATCH for c++/83911, ICE with multiversioned constructor
On Wed, Mar 14, 2018 at 8:57 PM, Jason Merrill wrote: > Ping > > On Fri, Mar 2, 2018 at 1:23 PM, Jason Merrill wrote: >> As I mentioned in the PR, the problem here is that we're replacing a >> constructor with a dispatcher function which doesn't look much like a >> constructor. This patch adjusts make_dispatcher_decl to make it look >> more like the functions it dispatches to, but other things are certain >> to break for similar reasons down the road. A proper solution should >> be more transparent, like thunks. >> >> Tested x86_64-pc-linux-gnu. Does this seem worth applying to fix the >> regression? The patch looks reasonable to me, you probably know best whether the cp/ parts are risky or not ;) So - OK from my POV. And yes, thunks may be a better representation for the dispatcher. Richard.
Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.
On Mon, Feb 26, 2018 at 12:07 PM, Richard Biener wrote: > On Fri, Feb 16, 2018 at 6:18 PM, wrote: >> From: Vladimir Mezentsev >> >> Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to >> resolve >> bootstrap comparison failure (2015-11-10, commit >> bc443a71dafa2e707bae4b2fa74f83b05dea37ab). >> The real bug is in gcc/varasm.c. >> hash_section() returns an unstable value. >> As result, two blocks are created in get_block_for_section() for one unnamed >> section. >> A list of objects in these blocks depends on the -gtoggle option. >> I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and >> I fixed hash_section() in gcc/varasm.c >> >> Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go). >> Testing finished ok. > > Ok. Committed on behalf of Vladimir. r258553. Richard. > Thanks, > Richard. > >> ChangeLog: >> 2018-02-15 Vladimir Mezentsev >> >> PR gcc/68256 >> * varasm.c (hash_section): Return an unchangeble hash value >> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p): >> Return !aarch64_can_use_per_function_literal_pools_p (); >> --- >> gcc/config/aarch64/aarch64.c | 8 +++- >> gcc/varasm.c | 2 +- >> 2 files changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >> index 174310c..a0a495d 100644 >> --- a/gcc/config/aarch64/aarch64.c >> +++ b/gcc/config/aarch64/aarch64.c >> @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void) >> static bool >> aarch64_use_blocks_for_constant_p (machine_mode, const_rtx) >> { >> - /* Fixme:: In an ideal world this would work similar >> - to the logic in aarch64_select_rtx_section but this >> - breaks bootstrap in gcc go. For now we workaround >> - this by returning false here. */ >> - return false; >> + /* We can't use blocks for constants when we're using a per-function >> + constant pool. */ >> + return !aarch64_can_use_per_function_literal_pools_p (); >> } >> >> /* Select appropriate section for constants depending >> diff --git a/gcc/varasm.c b/gcc/varasm.c >> index b045efa..5aae5b4 100644 >> --- a/gcc/varasm.c >> +++ b/gcc/varasm.c >> @@ -225,7 +225,7 @@ hash_section (section *sect) >> { >>if (sect->common.flags & SECTION_NAMED) >> return htab_hash_string (sect->named.name); >> - return sect->common.flags; >> + return sect->common.flags & ~SECTION_DECLARED; >> } >> >> /* Helper routines for maintaining object_block_htab. */ >> -- >> 1.8.3.1 >>
Re: [PATCH] Add test-case (PR ipa/84805).
On 03/14/2018 09:12 PM, Eric Botcazou wrote: >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Please make sure to test it on more platforms (see PR ipa/83983 for details). > Sorry for the breakage. Actually I did testing on x86_64 and ppcl64 machines. Martin
Re: [PATCH] Fortran -- clean up KILL
On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl wrote: > On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote: >> >> int val = kill (pid, signal); >> return (val == 0): 0 ? errno; >> >> like it already does for the optional status argument for kill_sub. >> > > Committed as r258511 with your suggested change. Hi Steve, After this change, AArch64/arm bare-metal cross (fortran) toolchain fail to build with below error message: /.../obj/gcc2/./gcc/xgcc -B/.../obj/gcc2/./gcc/ -B/.../install/aarch64-none-elf/bin/ -B/.../install/aarch64-none-elf/lib/ -isystem /.../install/aarch64-none-elf/include -isystem /.../install/aarch64-none-elf/sys-include -DHAVE_CONFIG_H -I. -I/.../gcc/libgfortran -iquote/.../gcc/libgfortran/io -I/.../gcc/libgfortran/../gcc -I/.../gcc/libgfortran/../gcc/config -I../../.././gcc -I/.../gcc/libgfortran/../libgcc -I../../libgcc -I/.../gcc/libgfortran/../libbacktrace -I../../libbacktrace -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules -ffunction-sections -fdata-sections -g -ffunction-sections -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types for 'kill' extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); ^~~~ In file included from /.../install/aarch64-none-elf/include/signal.h:6, from /.../gcc/libgfortran/intrinsics/kill.c:28: /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: previous declaration of 'kill' was here int kill (pid_t, int); ^~~~ In file included from /.../gcc/libgfortran/intrinsics/kill.c:26: /.../gcc/libgfortran/intrinsics/kill.c:55:14: error: conflicting types for 'kill' export_proto(kill); ^~~~ /.../gcc/libgfortran/libgfortran.h:150:57: note: in definition of macro 'sym_rename2' #define sym_rename2(old, ulp, new) extern __typeof(old) old __asm__(#ulp #new) ^~~ /.../gcc/libgfortran/libgfortran.h:148:30: note: in expansion of macro 'sym_rename1' #define sym_rename(old, new) sym_rename1(old, __USER_LABEL_PREFIX__, new) ^~~ /.../gcc/libgfortran/libgfortran.h:195:26: note: in expansion of macro 'sym_rename' # define export_proto(x) sym_rename(x, PREFIX(x)) ^~ /.../gcc/libgfortran/intrinsics/kill.c:55:1: note: in expansion of macro 'export_proto' export_proto(kill); ^~~~ In file included from /.../install/aarch64-none-elf/include/signal.h:6, from /.../gcc/libgfortran/intrinsics/kill.c:28: /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: previous declaration of 'kill' was here int kill (pid_t, int); ^~~~ /.../gcc/libgfortran/intrinsics/kill.c:58:1: error: conflicting types for 'kill' kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) ^~~~ In file included from /.../install/aarch64-none-elf/include/signal.h:6, from /.../gcc/libgfortran/intrinsics/kill.c:28: /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: previous declaration of 'kill' was here int kill (pid_t, int); ^~~~ The gcc is configured with: gcc/configure --target=aarch64-none-elf --prefix=... --with-gmp=.../host-tools --with-mpfr=.../host-tools --with-mpc=.../host-tools --with-isl=.../host-tools --with-pkgversion=unknown --disable-shared --disable-nls --disable-threads --disable-tls --enable-checking=yes --enable-languages=c,c++ --with-newlib --enable-languages=c,c++,fortran I don't know fortran, so any suggestion how to fix this? Note that -mabi=ilp32 is required to reproduce the breakage. Thanks, bin > > -- > Steve
Re: [PATCH][GCC][ARM] Fix can_change_mode_class for big-endian
Hi Tamar, On 05/03/18 16:51, Tamar Christina wrote: Hi All, Taking the subreg of a vector mode on big-endian may result in an infinite recursion and eventually a segfault once we run out of stack space. As an example, taking a subreg of V4HF to SImode we end up in the following loop on big-endian: #861 0x008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787 #862 0x00882a90 in emit_move_multi_word src/gcc/gcc/expr.c:3621 #863 0x0087eea1 in emit_move_insn_1 src/gcc/gcc/expr.c:3698 #864 0x0087f350 in emit_move_insn src/gcc/gcc/expr.c:3757 #865 0x0085e326 in copy_to_reg src/gcc/gcc/explow.c:603 #866 0x008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787 The reason is that operand_subword_force will always fail. When the value is in a register that can't be accessed as a multi word the code tries to create a new psuedo register and emit the value to it. Eventually you end up in simplify_gen_subreg which calls validate_subreg. validate_subreg will however always fail because of the REG_CAN_CHANGE_MODE_P check. On little endian this check always returns true. On big-endian this check is supposed to prevent values that have a size larger than word size, due to those being stored in VFP registers. However we are only interested in a subreg of the vector mode, so we should be checking the unit size, not the size of the entire mode. Doing this fixes the problem. Regtested on armeb-none-eabi and no regressions. Bootstrapped on arm-none-linux-gnueabihf and no issues. Ok for trunk? and for backport to GCC 7? Ok for trunk. Please wait for a few days before backporting. Thanks, Kyrill Thanks, Tamar gcc/ 2018-03-05 Tamar Christina PR target/84711 * config/arm/arm.c (arm_can_change_mode_class): Use GET_MODE_UNIT_SIZE instead of GET_MODE_SIZE when comparing Units. gcc/testsuite/ 2018-03-05 Tamar Christina PR target/84711 * gcc.target/arm/big-endian-subreg.c: New. --
[arm-embedded][PATCH] Add multilib mapping for -mcpu=cortex-m33+nodsp
Hi, Currently -mcpu=cortex-m33+nodsp gets assigned the thumb multilib due to lack of mapping from -mcpu=cortex-m33+nodsp to an -march option. This leads to link failures for linking Armv4T Thumb code from the multilib with Armv8-M Mainline code from the code being compiled. This patch adds the appropriate mapping. ChangeLog entry is as follows: *** gcc/ChangeLog.arm *** 2018-03-14 Thomas Preud'homme * config/arm/t-rmprofile: Add mapping from -mcpu=cortex-m33+nodsp to -march=armv8-m.main. Testing: A hello world fails to link without the patch with a multilib build but succeeds with the patch. We've decided to apply this patch to the ARM/embedded-7-branch branch. Best regards, Thomas diff --git a/gcc/config/arm/t-rmprofile b/gcc/config/arm/t-rmprofile index a3a24d59fb29b42a36177bd2d2ebfae4e50e5a10..54411795215b8aff90ba9cfb806ec7b33db4caea 100644 --- a/gcc/config/arm/t-rmprofile +++ b/gcc/config/arm/t-rmprofile @@ -102,6 +102,7 @@ MULTILIB_MATCHES += march?armv7e-m=mcpu?cortex-m4 MULTILIB_MATCHES += march?armv7e-m=mcpu?cortex-m7 MULTILIB_MATCHES += march?armv8-m.base=mcpu?cortex-m23 MULTILIB_MATCHES += march?armv8-m.main=mcpu?cortex-m33 +MULTILIB_MATCHES += march?armv8-m.main=mcpu?cortex-m33+nodsp MULTILIB_MATCHES += march?armv7=mcpu?cortex-r4 MULTILIB_MATCHES += march?armv7=mcpu?cortex-r4f MULTILIB_MATCHES += march?armv7=mcpu?cortex-r5
Re: C++ PATCH to fix an ICE on invalid with OVERLOADs (PR c++/84854)
On Wed, Mar 14, 2018 at 02:06:36PM -0400, Jason Merrill wrote: > On Wed, Mar 14, 2018 at 11:59 AM, Marek Polacek wrote: > > cxx_constant_value doesn't understand template codes, and neither it > > understands OVERLOADs, so if we pass an OVERLOAD to it, we crash. Here > > instantiate_non_dependent_expr got an OVERLOAD, but since it calls > > is_nondependent_constant_expression which checks type_unknown_p, it left the > > expression as it was. We can't use is_nondependent_constant_expression in > > finish_if_stmt_cond because i_n_c_e checks is_constant_expression and that > > is > > not suitable here; we'd miss diagnostics. So I did the following; I think > > we > > should reject the testcase with an error. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > 2018-03-14 Marek Polacek > > > > PR c++/84854 > > * semantics.c (finish_if_stmt_cond): Give error if the condition > > is an overloaded function with no contextual type information. > > > > * g++.dg/cpp1z/constexpr-if15.C: New test. > > > > diff --git gcc/cp/semantics.c gcc/cp/semantics.c > > index fdf37bea770..a056e9445e9 100644 > > --- gcc/cp/semantics.c > > +++ gcc/cp/semantics.c > > @@ -735,8 +735,16 @@ finish_if_stmt_cond (tree cond, tree if_stmt) > >&& require_constant_expression (cond) > >&& !value_dependent_expression_p (cond)) > > { > > - cond = instantiate_non_dependent_expr (cond); > > - cond = cxx_constant_value (cond, NULL_TREE); > > + if (type_unknown_p (cond)) > > + { > > + cxx_incomplete_type_error (cond, TREE_TYPE (cond)); > > + cond = error_mark_node; > > I think I'd prefer to skip this block when type_unknown_p, and leave > error handling up to the code shared with regular if. Like this? It was my first version, but I thought we wanted the error; the following rejects the testcase only when instantiating the template function. Which is fine by me, too. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-03-15 Marek Polacek PR c++/84854 * semantics.c (finish_if_stmt_cond): Check type_unknown_p. * g++.dg/cpp1z/constexpr-if15.C: New test. diff --git gcc/cp/semantics.c gcc/cp/semantics.c index fdf37bea770..753a5f4d4f8 100644 --- gcc/cp/semantics.c +++ gcc/cp/semantics.c @@ -733,7 +733,8 @@ finish_if_stmt_cond (tree cond, tree if_stmt) if (IF_STMT_CONSTEXPR_P (if_stmt) && !type_dependent_expression_p (cond) && require_constant_expression (cond) - && !value_dependent_expression_p (cond)) + && !value_dependent_expression_p (cond) + && !type_unknown_p (cond)) { cond = instantiate_non_dependent_expr (cond); cond = cxx_constant_value (cond, NULL_TREE); diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-if15.C gcc/testsuite/g++.dg/cpp1z/constexpr-if15.C index e69de29bb2d..c819b3e3a07 100644 --- gcc/testsuite/g++.dg/cpp1z/constexpr-if15.C +++ gcc/testsuite/g++.dg/cpp1z/constexpr-if15.C @@ -0,0 +1,11 @@ +// PR c++/84854 +// { dg-options -std=c++17 } + +constexpr int foo () { return 1; } +constexpr int foo (int) { return 2; } + +template +void a() +{ + if constexpr(foo) { }; +} Marek
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng wrote: > On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl > wrote: >> On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote: >>> >>> int val = kill (pid, signal); >>> return (val == 0): 0 ? errno; >>> >>> like it already does for the optional status argument for kill_sub. >>> >> >> Committed as r258511 with your suggested change. > Hi Steve, > > After this change, AArch64/arm bare-metal cross (fortran) toolchain > fail to build with below error message: > > /.../obj/gcc2/./gcc/xgcc -B/.../obj/gcc2/./gcc/ > -B/.../install/aarch64-none-elf/bin/ > -B/.../install/aarch64-none-elf/lib/ -isystem > /.../install/aarch64-none-elf/include -isystem > /.../install/aarch64-none-elf/sys-include -DHAVE_CONFIG_H -I. > -I/.../gcc/libgfortran -iquote/.../gcc/libgfortran/io > -I/.../gcc/libgfortran/../gcc -I/.../gcc/libgfortran/../gcc/config > -I../../.././gcc -I/.../gcc/libgfortran/../libgcc -I../../libgcc > -I/.../gcc/libgfortran/../libbacktrace -I../../libbacktrace > -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes > -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings > -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules > -ffunction-sections -fdata-sections -g -ffunction-sections > -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo > -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o > /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types > for 'kill' > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > ^~~~ > In file included from /.../install/aarch64-none-elf/include/signal.h:6, > from /.../gcc/libgfortran/intrinsics/kill.c:28: > /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: > previous declaration of 'kill' was here > int kill (pid_t, int); > ^~~~ > In file included from /.../gcc/libgfortran/intrinsics/kill.c:26: > /.../gcc/libgfortran/intrinsics/kill.c:55:14: error: conflicting types > for 'kill' > export_proto(kill); > ^~~~ > /.../gcc/libgfortran/libgfortran.h:150:57: note: in definition of > macro 'sym_rename2' > #define sym_rename2(old, ulp, new) extern __typeof(old) old __asm__(#ulp > #new) > ^~~ > /.../gcc/libgfortran/libgfortran.h:148:30: note: in expansion of macro > 'sym_rename1' > #define sym_rename(old, new) sym_rename1(old, __USER_LABEL_PREFIX__, new) > ^~~ > /.../gcc/libgfortran/libgfortran.h:195:26: note: in expansion of macro > 'sym_rename' > # define export_proto(x) sym_rename(x, PREFIX(x)) > ^~ > /.../gcc/libgfortran/intrinsics/kill.c:55:1: note: in expansion of > macro 'export_proto' > export_proto(kill); > ^~~~ > In file included from /.../install/aarch64-none-elf/include/signal.h:6, > from /.../gcc/libgfortran/intrinsics/kill.c:28: > /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: > previous declaration of 'kill' was here > int kill (pid_t, int); > ^~~~ > /.../gcc/libgfortran/intrinsics/kill.c:58:1: error: conflicting types for > 'kill' > kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) > ^~~~ > In file included from /.../install/aarch64-none-elf/include/signal.h:6, > from /.../gcc/libgfortran/intrinsics/kill.c:28: > /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: > previous declaration of 'kill' was here > int kill (pid_t, int); > ^~~~ > > The gcc is configured with: > gcc/configure --target=aarch64-none-elf --prefix=... > --with-gmp=.../host-tools --with-mpfr=.../host-tools > --with-mpc=.../host-tools --with-isl=.../host-tools > --with-pkgversion=unknown --disable-shared --disable-nls > --disable-threads --disable-tls --enable-checking=yes > --enable-languages=c,c++ --with-newlib > --enable-languages=c,c++,fortran > > I don't know fortran, so any suggestion how to fix this? > Note that -mabi=ilp32 is required to reproduce the breakage. So the pre-processed file for libgfortran/intrinsics/kill.c is like: int kill (pid_t, int); //.. extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); extern __typeof(kill) kill __asm__("" "_gfortran_kill"); GFC_INTEGER_4 kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) { int val; val = (int)kill (pid, signal); return ((val == 0) ? 0 : # 62 "/.../gcc/libgfortran/intrinsics/kill.c" 3 4 (*__errno()) # 62 "/.../gcc/libgfortran/intrinsics/kill.c" ); } I suppose fortran wants to define its own kill returning errorno directly on failure. The problem is below declaration: extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); could conflict with included c declaration which is: int kill (pid_t, int); Even GFC_INTEGER_4 is typedef as int32_t, we can't rely on that is the same as int type, right? In this case, int32_t is defined as long int on aarch64_ilp32 or a
Re: [PATCH] Add test-case (PR ipa/84805).
> Sorry for the breakage. Actually I did testing on x86_64 and ppcl64 > machines. There is no breakage, just an issue with the ordering of warnings which may vary across platforms. -- Eric Botcazou
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 12:11 PM, Bin.Cheng wrote: > On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng wrote: >> On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl >> wrote: >>> On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote: int val = kill (pid, signal); return (val == 0): 0 ? errno; like it already does for the optional status argument for kill_sub. >>> >>> Committed as r258511 with your suggested change. >> Hi Steve, >> >> After this change, AArch64/arm bare-metal cross (fortran) toolchain >> fail to build with below error message: >> >> /.../obj/gcc2/./gcc/xgcc -B/.../obj/gcc2/./gcc/ >> -B/.../install/aarch64-none-elf/bin/ >> -B/.../install/aarch64-none-elf/lib/ -isystem >> /.../install/aarch64-none-elf/include -isystem >> /.../install/aarch64-none-elf/sys-include -DHAVE_CONFIG_H -I. >> -I/.../gcc/libgfortran -iquote/.../gcc/libgfortran/io >> -I/.../gcc/libgfortran/../gcc -I/.../gcc/libgfortran/../gcc/config >> -I../../.././gcc -I/.../gcc/libgfortran/../libgcc -I../../libgcc >> -I/.../gcc/libgfortran/../libbacktrace -I../../libbacktrace >> -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes >> -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings >> -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules >> -ffunction-sections -fdata-sections -g -ffunction-sections >> -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo >> -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o >> /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types >> for 'kill' >> extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); >> ^~~~ >> In file included from /.../install/aarch64-none-elf/include/signal.h:6, >> from /.../gcc/libgfortran/intrinsics/kill.c:28: >> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: >> previous declaration of 'kill' was here >> int kill (pid_t, int); >> ^~~~ >> In file included from /.../gcc/libgfortran/intrinsics/kill.c:26: >> /.../gcc/libgfortran/intrinsics/kill.c:55:14: error: conflicting types >> for 'kill' >> export_proto(kill); >> ^~~~ >> /.../gcc/libgfortran/libgfortran.h:150:57: note: in definition of >> macro 'sym_rename2' >> #define sym_rename2(old, ulp, new) extern __typeof(old) old __asm__(#ulp >> #new) >> ^~~ >> /.../gcc/libgfortran/libgfortran.h:148:30: note: in expansion of macro >> 'sym_rename1' >> #define sym_rename(old, new) sym_rename1(old, __USER_LABEL_PREFIX__, new) >> ^~~ >> /.../gcc/libgfortran/libgfortran.h:195:26: note: in expansion of macro >> 'sym_rename' >> # define export_proto(x) sym_rename(x, PREFIX(x)) >> ^~ >> /.../gcc/libgfortran/intrinsics/kill.c:55:1: note: in expansion of >> macro 'export_proto' >> export_proto(kill); >> ^~~~ >> In file included from /.../install/aarch64-none-elf/include/signal.h:6, >> from /.../gcc/libgfortran/intrinsics/kill.c:28: >> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: >> previous declaration of 'kill' was here >> int kill (pid_t, int); >> ^~~~ >> /.../gcc/libgfortran/intrinsics/kill.c:58:1: error: conflicting types for >> 'kill' >> kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) >> ^~~~ >> In file included from /.../install/aarch64-none-elf/include/signal.h:6, >> from /.../gcc/libgfortran/intrinsics/kill.c:28: >> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: >> previous declaration of 'kill' was here >> int kill (pid_t, int); >> ^~~~ >> >> The gcc is configured with: >> gcc/configure --target=aarch64-none-elf --prefix=... >> --with-gmp=.../host-tools --with-mpfr=.../host-tools >> --with-mpc=.../host-tools --with-isl=.../host-tools >> --with-pkgversion=unknown --disable-shared --disable-nls >> --disable-threads --disable-tls --enable-checking=yes >> --enable-languages=c,c++ --with-newlib >> --enable-languages=c,c++,fortran >> >> I don't know fortran, so any suggestion how to fix this? >> Note that -mabi=ilp32 is required to reproduce the breakage. > > So the pre-processed file for libgfortran/intrinsics/kill.c is like: > > > int kill (pid_t, int); > > //.. > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > extern __typeof(kill) kill __asm__("" "_gfortran_kill"); > > GFC_INTEGER_4 > kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) > { > int val; > val = (int)kill (pid, signal); > return ((val == 0) ? 0 : > # 62 "/.../gcc/libgfortran/intrinsics/kill.c" 3 4 > (*__errno()) > # 62 "/.../gcc/libgfortran/intrinsics/kill.c" >); > } > > I suppose fortran wants to define its own kill returning errorno > directly on failure. > The problem is below declaration: > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > > could conflict with included c declaration w
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 1:11 PM, Bin.Cheng wrote: > On Thu, Mar 15, 2018 at 10:18 AM, Bin.Cheng wrote: >> On Wed, Mar 14, 2018 at 12:57 AM, Steve Kargl >> wrote: >>> On Tue, Mar 13, 2018 at 09:49:10PM +0200, Janne Blomqvist wrote: int val = kill (pid, signal); return (val == 0): 0 ? errno; like it already does for the optional status argument for kill_sub. >>> >>> Committed as r258511 with your suggested change. >> Hi Steve, >> >> After this change, AArch64/arm bare-metal cross (fortran) toolchain >> fail to build with below error message: >> >> /.../obj/gcc2/./gcc/xgcc -B/.../obj/gcc2/./gcc/ >> -B/.../install/aarch64-none-elf/bin/ >> -B/.../install/aarch64-none-elf/lib/ -isystem >> /.../install/aarch64-none-elf/include -isystem >> /.../install/aarch64-none-elf/sys-include -DHAVE_CONFIG_H -I. >> -I/.../gcc/libgfortran -iquote/.../gcc/libgfortran/io >> -I/.../gcc/libgfortran/../gcc -I/.../gcc/libgfortran/../gcc/config >> -I../../.././gcc -I/.../gcc/libgfortran/../libgcc -I../../libgcc >> -I/.../gcc/libgfortran/../libbacktrace -I../../libbacktrace >> -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes >> -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings >> -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules >> -ffunction-sections -fdata-sections -g -ffunction-sections >> -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo >> -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o >> /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types >> for 'kill' >> extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); >> ^~~~ >> In file included from /.../install/aarch64-none-elf/include/signal.h:6, >> from /.../gcc/libgfortran/intrinsics/kill.c:28: >> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: >> previous declaration of 'kill' was here >> int kill (pid_t, int); >> ^~~~ >> In file included from /.../gcc/libgfortran/intrinsics/kill.c:26: >> /.../gcc/libgfortran/intrinsics/kill.c:55:14: error: conflicting types >> for 'kill' >> export_proto(kill); >> ^~~~ >> /.../gcc/libgfortran/libgfortran.h:150:57: note: in definition of >> macro 'sym_rename2' >> #define sym_rename2(old, ulp, new) extern __typeof(old) old __asm__(#ulp >> #new) >> ^~~ >> /.../gcc/libgfortran/libgfortran.h:148:30: note: in expansion of macro >> 'sym_rename1' >> #define sym_rename(old, new) sym_rename1(old, __USER_LABEL_PREFIX__, new) >> ^~~ >> /.../gcc/libgfortran/libgfortran.h:195:26: note: in expansion of macro >> 'sym_rename' >> # define export_proto(x) sym_rename(x, PREFIX(x)) >> ^~ >> /.../gcc/libgfortran/intrinsics/kill.c:55:1: note: in expansion of >> macro 'export_proto' >> export_proto(kill); >> ^~~~ >> In file included from /.../install/aarch64-none-elf/include/signal.h:6, >> from /.../gcc/libgfortran/intrinsics/kill.c:28: >> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: >> previous declaration of 'kill' was here >> int kill (pid_t, int); >> ^~~~ >> /.../gcc/libgfortran/intrinsics/kill.c:58:1: error: conflicting types for >> 'kill' >> kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) >> ^~~~ >> In file included from /.../install/aarch64-none-elf/include/signal.h:6, >> from /.../gcc/libgfortran/intrinsics/kill.c:28: >> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: >> previous declaration of 'kill' was here >> int kill (pid_t, int); >> ^~~~ >> >> The gcc is configured with: >> gcc/configure --target=aarch64-none-elf --prefix=... >> --with-gmp=.../host-tools --with-mpfr=.../host-tools >> --with-mpc=.../host-tools --with-isl=.../host-tools >> --with-pkgversion=unknown --disable-shared --disable-nls >> --disable-threads --disable-tls --enable-checking=yes >> --enable-languages=c,c++ --with-newlib >> --enable-languages=c,c++,fortran >> >> I don't know fortran, so any suggestion how to fix this? >> Note that -mabi=ilp32 is required to reproduce the breakage. > > So the pre-processed file for libgfortran/intrinsics/kill.c is like: > > > int kill (pid_t, int); > > //.. > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > extern __typeof(kill) kill __asm__("" "_gfortran_kill"); Why do you need to jump through these hoops anyway? Just do ... > GFC_INTEGER_4 > kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) _gfortran_kill (... here. The kill prototype should come via signal.h already. I guess the issue is that libgfortran 'kill' (exported with _gfortran_ prefix) aliases the POSIX kill and you even want to call that here. So maybe just use export_proto_np or add another variant that works on function definitions directly so you can write GFC_INTEGER_4 export_def(kill) (GFC_INTEGER_4 ...) { } > { > int val; > val = (int)kill
Re: C++ PATCH to fix an ICE on invalid with OVERLOADs (PR c++/84854)
On Thu, Mar 15, 2018 at 7:49 AM, Marek Polacek wrote: > On Wed, Mar 14, 2018 at 02:06:36PM -0400, Jason Merrill wrote: >> On Wed, Mar 14, 2018 at 11:59 AM, Marek Polacek wrote: >> > cxx_constant_value doesn't understand template codes, and neither it >> > understands OVERLOADs, so if we pass an OVERLOAD to it, we crash. Here >> > instantiate_non_dependent_expr got an OVERLOAD, but since it calls >> > is_nondependent_constant_expression which checks type_unknown_p, it left >> > the >> > expression as it was. We can't use is_nondependent_constant_expression in >> > finish_if_stmt_cond because i_n_c_e checks is_constant_expression and that >> > is >> > not suitable here; we'd miss diagnostics. So I did the following; I think >> > we >> > should reject the testcase with an error. >> > >> > Bootstrapped/regtested on x86_64-linux, ok for trunk? >> > >> > 2018-03-14 Marek Polacek >> > >> > PR c++/84854 >> > * semantics.c (finish_if_stmt_cond): Give error if the condition >> > is an overloaded function with no contextual type information. >> > >> > * g++.dg/cpp1z/constexpr-if15.C: New test. >> > >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c >> > index fdf37bea770..a056e9445e9 100644 >> > --- gcc/cp/semantics.c >> > +++ gcc/cp/semantics.c >> > @@ -735,8 +735,16 @@ finish_if_stmt_cond (tree cond, tree if_stmt) >> >&& require_constant_expression (cond) >> >&& !value_dependent_expression_p (cond)) >> > { >> > - cond = instantiate_non_dependent_expr (cond); >> > - cond = cxx_constant_value (cond, NULL_TREE); >> > + if (type_unknown_p (cond)) >> > + { >> > + cxx_incomplete_type_error (cond, TREE_TYPE (cond)); >> > + cond = error_mark_node; >> >> I think I'd prefer to skip this block when type_unknown_p, and leave >> error handling up to the code shared with regular if. > > Like this? Yes, thanks. > It was my first version, but I thought we wanted the error; Getting an error is an improvement, but it should apply to non-constexpr if as well, so checking in maybe_convert_cond would be better. Actually, if we deal with unknown type there, we shouldn't need this patch at all. ...except I also notice that since maybe_convert_cond doesn't do any conversion in a template, we're trying to extract the constant value without first converting to bool, which breaks this testcase: struct A { constexpr operator bool () { return true; } int i; }; A a; template void f() { constexpr bool b = a; // works if constexpr (a) { } // breaks } int main() { f(); } A simple fix would be to replace your type_unknown_p check here with a comparison to boolean_type_node, so we wait until instantiation time to require a constant value. Better would be to actually do the conversion. Perhaps this could share code (for converting and getting a constant value) with finish_static_assert. Jason
[PATCH] Fix PR84873
The following fixes the C familiy gimplification langhook to not introduce tree sharing which isn't valid during gimplification. For the specific case the tree sharing is introduced by fold_binary_op_with_cond and is reached via convert () eventually folding something. I've kept folding constants here but for the rest defer folding to GIMPLE (the gimplifier already folds most generated stmts). Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk and branches? Thanks, Richard. 2018-03-15 Richard Biener PR c/84873 * c-gimplify.c (c_gimplify_expr): Do not fold expressions. * c-c++-common/pr84873.c: New testcase. Index: gcc/c-family/c-gimplify.c === --- gcc/c-family/c-gimplify.c (revision 258552) +++ gcc/c-family/c-gimplify.c (working copy) @@ -245,7 +245,15 @@ c_gimplify_expr (tree *expr_p, gimple_se unsigned_type_node) && !types_compatible_p (TYPE_MAIN_VARIANT (TREE_TYPE (*op1_p)), integer_type_node)) - *op1_p = convert (unsigned_type_node, *op1_p); + { + /* ??? Do not use convert () here or fold arbitrary trees + since folding can introduce tree sharing which is not + allowed during gimplification. */ + if (TREE_CODE (*op1_p) == INTEGER_CST) + *op1_p = fold_convert (unsigned_type_node, *op1_p); + else + *op1_p = build1 (NOP_EXPR, unsigned_type_node, *op1_p); + } break; } Index: gcc/testsuite/c-c++-common/pr84873.c === --- gcc/testsuite/c-c++-common/pr84873.c(nonexistent) +++ gcc/testsuite/c-c++-common/pr84873.c(working copy) @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-frounding-math" } */ + +int +i1 (int w3, int n9) +{ + return w3 >> ((long int)(1 + 0.1) + -!n9); +}
Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.
On Fri, Feb 16, 2018 at 5:18 PM, wrote: > From: Vladimir Mezentsev > > Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to > resolve > bootstrap comparison failure (2015-11-10, commit > bc443a71dafa2e707bae4b2fa74f83b05dea37ab). > The real bug is in gcc/varasm.c. > hash_section() returns an unstable value. > As result, two blocks are created in get_block_for_section() for one unnamed > section. > A list of objects in these blocks depends on the -gtoggle option. > I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and > I fixed hash_section() in gcc/varasm.c > > Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go). > Testing finished ok. Hi Vladimir, Thanks for fixing the long standing issue, but this change causes below failure, could you have a look? Thanks Failures: gcc.dg/attr-weakref-1.c Bisected to: commit 536728c16d6a0173930ecfe370302baa471c299e Author: rguenth Date: Thu Mar 15 08:55:04 2018 + 2018-03-15 Vladimir Mezentsev PR target/68256 * varasm.c (hash_section): Return an unchangeble hash value * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p): Return !aarch64_can_use_per_function_literal_pools_p (). Thanks, bin > > ChangeLog: > 2018-02-15 Vladimir Mezentsev > > PR gcc/68256 > * varasm.c (hash_section): Return an unchangeble hash value > * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p): > Return !aarch64_can_use_per_function_literal_pools_p (); > --- > gcc/config/aarch64/aarch64.c | 8 +++- > gcc/varasm.c | 2 +- > 2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 174310c..a0a495d 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void) > static bool > aarch64_use_blocks_for_constant_p (machine_mode, const_rtx) > { > - /* Fixme:: In an ideal world this would work similar > - to the logic in aarch64_select_rtx_section but this > - breaks bootstrap in gcc go. For now we workaround > - this by returning false here. */ > - return false; > + /* We can't use blocks for constants when we're using a per-function > + constant pool. */ > + return !aarch64_can_use_per_function_literal_pools_p (); > } > > /* Select appropriate section for constants depending > diff --git a/gcc/varasm.c b/gcc/varasm.c > index b045efa..5aae5b4 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -225,7 +225,7 @@ hash_section (section *sect) > { >if (sect->common.flags & SECTION_NAMED) > return htab_hash_string (sect->named.name); > - return sect->common.flags; > + return sect->common.flags & ~SECTION_DECLARED; > } > > /* Helper routines for maintaining object_block_htab. */ > -- > 1.8.3.1 >
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 01:35:23PM +0100, Richard Biener wrote: > > > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > > extern __typeof(kill) kill __asm__("" "_gfortran_kill"); > > Why do you need to jump through these hoops anyway? Just do ... > Not sure who the "you" refers to. The easiest fix be appending a 4 to kill. I'll do that later. -- Steve
Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.
On 15 March 2018 at 15:07, Bin.Cheng wrote: > On Fri, Feb 16, 2018 at 5:18 PM, wrote: >> From: Vladimir Mezentsev >> >> Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to >> resolve >> bootstrap comparison failure (2015-11-10, commit >> bc443a71dafa2e707bae4b2fa74f83b05dea37ab). >> The real bug is in gcc/varasm.c. >> hash_section() returns an unstable value. >> As result, two blocks are created in get_block_for_section() for one unnamed >> section. >> A list of objects in these blocks depends on the -gtoggle option. >> I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and >> I fixed hash_section() in gcc/varasm.c >> >> Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go). >> Testing finished ok. > Hi Vladimir, > Thanks for fixing the long standing issue, but this change causes below > failure, > could you have a look? Thanks > > Failures: > gcc.dg/attr-weakref-1.c > > Bisected to: > > > commit 536728c16d6a0173930ecfe370302baa471c299e > Author: rguenth > Date: Thu Mar 15 08:55:04 2018 + > > 2018-03-15 Vladimir Mezentsev > > PR target/68256 > * varasm.c (hash_section): Return an unchangeble hash value > * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p): > Return !aarch64_can_use_per_function_literal_pools_p (). > I see this too: /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/gcc/xgcc -B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/gcc/ /gcc/testsuite/gcc.dg/attr-weakref-1.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O2 /gcc/testsuite/gcc.dg/attr-weakref-1a.c -lm -o ./attr-weakref-1.exe /ccLqgn8f.o:(.rodata.cst8+0x30): undefined reference to `wv12' /ccLqgn8f.o:(.rodata.cst8+0x38): undefined reference to `wv12' /ccLqgn8f.o:(.rodata.cst8+0x60): undefined reference to `wf12' /ccLqgn8f.o:(.rodata.cst8+0x68): undefined reference to `wf12' collect2: error: ld returned 1 exit status compiler exited with status 1 FAIL: gcc.dg/attr-weakref-1.c (test for excess errors) Christophe > > Thanks, > bin >> >> ChangeLog: >> 2018-02-15 Vladimir Mezentsev >> >> PR gcc/68256 >> * varasm.c (hash_section): Return an unchangeble hash value >> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p): >> Return !aarch64_can_use_per_function_literal_pools_p (); >> --- >> gcc/config/aarch64/aarch64.c | 8 +++- >> gcc/varasm.c | 2 +- >> 2 files changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >> index 174310c..a0a495d 100644 >> --- a/gcc/config/aarch64/aarch64.c >> +++ b/gcc/config/aarch64/aarch64.c >> @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void) >> static bool >> aarch64_use_blocks_for_constant_p (machine_mode, const_rtx) >> { >> - /* Fixme:: In an ideal world this would work similar >> - to the logic in aarch64_select_rtx_section but this >> - breaks bootstrap in gcc go. For now we workaround >> - this by returning false here. */ >> - return false; >> + /* We can't use blocks for constants when we're using a per-function >> + constant pool. */ >> + return !aarch64_can_use_per_function_literal_pools_p (); >> } >> >> /* Select appropriate section for constants depending >> diff --git a/gcc/varasm.c b/gcc/varasm.c >> index b045efa..5aae5b4 100644 >> --- a/gcc/varasm.c >> +++ b/gcc/varasm.c >> @@ -225,7 +225,7 @@ hash_section (section *sect) >> { >>if (sect->common.flags & SECTION_NAMED) >> return htab_hash_string (sect->named.name); >> - return sect->common.flags; >> + return sect->common.flags & ~SECTION_DECLARED; >> } >> >> /* Helper routines for maintaining object_block_htab. */ >> -- >> 1.8.3.1 >>
[PATCH,rs6000] PR 844422 Fix FCTID, FCTIW with -mcpu=power7
GCC Maintainers: The following patch fixes an ICE when compiling the test case gcc -mcpu=power7 builtin-fctid-fctiw-runnable.c The GCC compiler now gives a message "error: builtin function ‘__builtin_fctiw’ requires the ‘-mpower8- vector’ option" and exits without generating an internal error. The patch has been tested by compiling by hand as given above. The regression testing has also been done on powerpc64-unknown-linux-gnu (Power 8 BE) powerpc64le-unknown-linux-gnu (Power 8 LE) powerpc64le-unknown-linux-gnu (Power 9 LE) with no regressions. Let me know if the patch looks OK or not. Thanks. Carl Love - gcc/ChangeLog: 2018-03-15 Carl Love PR target/84422 * config/rs6000/rs6000-builtin.def: Change macro expansion for FCTID, FCTIW to BU_P8V_MISC_1. --- gcc/config/rs6000/rs6000-builtin.def | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def index 783ecff..9925364 100644 --- a/gcc/config/rs6000/rs6000-builtin.def +++ b/gcc/config/rs6000/rs6000-builtin.def @@ -658,6 +658,14 @@ /* Miscellaneous builtins for instructions added in ISA 2.07. These instructions do require the ISA 2.07 vector support, but they aren't vector instructions. */ +#define BU_P8V_MISC_1(ENUM, NAME, ATTR, ICODE) \ + RS6000_BUILTIN_1 (MISC_BUILTIN_ ## ENUM, /* ENUM */ \ + "__builtin_" NAME, /* NAME */ \ + RS6000_BTM_P8_VECTOR, /* MASK */ \ + (RS6000_BTC_ ## ATTR/* ATTR */ \ +| RS6000_BTC_UNARY), \ + CODE_FOR_ ## ICODE) /* ICODE */ + #define BU_P8V_MISC_3(ENUM, NAME, ATTR, ICODE) \ RS6000_BUILTIN_3 (MISC_BUILTIN_ ## ENUM, /* ENUM */ \ "__builtin_" NAME, /* NAME */ \ @@ -1881,8 +1889,8 @@ BU_VSX_OVERLOAD_X (XST,"xst") BU_VSX_OVERLOAD_X (XST_BE, "xst_be") /* 1 argument builtins pre ISA 2.04. */ -BU_FP_MISC_1 (FCTID, "fctid",CONST, lrintdfdi2) -BU_FP_MISC_1 (FCTIW, "fctiw",CONST, lrintsfsi2) +BU_P8V_MISC_1 (FCTID, "fctid",CONST, lrintdfdi2) +BU_P8V_MISC_1 (FCTIW, "fctiw",CONST, lrintsfsi2) /* 2 argument CMPB instructions added in ISA 2.05. */ BU_P6_2 (CMPB_32,"cmpb_32",CONST, cmpbsi3) -- 2.7.4
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 12:20:08PM +, Bin.Cheng wrote: > >> -fdata-sections -O2 -mabi=ilp32 -MT kill.lo -MD -MP -MF .deps/kill.Tpo > >> -c /.../gcc/libgfortran/intrinsics/kill.c -o kill.o > >> /.../gcc/libgfortran/intrinsics/kill.c:54:22: error: conflicting types > >> for 'kill' > >> extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > >> ^~~~ > >> In file included from /.../install/aarch64-none-elf/include/signal.h:6, > >> from /.../gcc/libgfortran/intrinsics/kill.c:28: > >> /.../install/aarch64-none-elf/include/sys/signal.h:176:5: note: > >> previous declaration of 'kill' was here > >> int kill (pid_t, int); > >> ^~~~ Does this fix the issue for you? Index: libgfortran/intrinsics/kill.c === --- libgfortran/intrinsics/kill.c (revision 258537) +++ libgfortran/intrinsics/kill.c (working copy) @@ -36,11 +36,9 @@ see the files COPYING3 and COPYING.RUNTIME respectivel INTEGER, INTENT(IN) :: PID, SIGNAL */ #ifdef HAVE_KILL -extern void kill_sub (GFC_INTEGER_4, GFC_INTEGER_4, GFC_INTEGER_4 *); -iexport_proto(kill_sub); void -kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal, GFC_INTEGER_4 *status) +_gfortran_kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal, GFC_INTEGER_4 *status) { int val; @@ -49,13 +47,9 @@ kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal, GFC if (status != NULL) *status = (val == 0) ? 0 : errno; } -iexport(kill_sub); -extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); -export_proto(kill); - GFC_INTEGER_4 -kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) +_gfortran_kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) { int val; val = (int)kill (pid, signal); -- Steve 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4 20161221 https://www.youtube.com/watch?v=IbCHE-hONow
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 07:10:26AM -0700, Steve Kargl wrote: > On Thu, Mar 15, 2018 at 01:35:23PM +0100, Richard Biener wrote: > > > > > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > > > extern __typeof(kill) kill __asm__("" "_gfortran_kill"); > > > > Why do you need to jump through these hoops anyway? Just do ... > > > > Not sure who the "you" refers to. The easiest > fix be appending a 4 to kill. I'll do that > later. I think this is even easier, no need to rename anything: 2018-03-15 Jakub Jelinek PR libgfortran/84880 * intrinsics/kill.c (kill): Rename to... (PREFIX (kill)): ... this. Use export_proto_np instead of export_proto. --- libgfortran/intrinsics/kill.c.jj2018-03-14 09:44:57.988975360 +0100 +++ libgfortran/intrinsics/kill.c 2018-03-15 16:01:02.725668658 +0100 @@ -51,11 +51,11 @@ kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER } iexport(kill_sub); -extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); -export_proto(kill); +extern GFC_INTEGER_4 PREFIX (kill) (GFC_INTEGER_4, GFC_INTEGER_4); +export_proto_np(PREFIX (kill)); GFC_INTEGER_4 -kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) +PREFIX (kill) (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) { int val; val = (int)kill (pid, signal); Jakub
Re: PING
> > https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00577.html OK. > https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00587.html I think here problem happens earlier becuase we should not have profile in the function at first place. The change itself make sense, but lets debug who and how computes the profile :) > https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00590.html OK, thanks! Honza > > Diky, > Martin >
[PATCH] rs6000: Fix for the previous abi_v4_pass_in_fpr change
I was a bit over-enthusiastic, we still support xilinxfp. Tested on powerpc64-linux {-m664,-m32}, committing. Segher 2018-03-15 Segher Boessenkool * config/rs6000/rs6000.c (abi_v4_pass_in_fpr): Add back the TARGET_DOUBLE_FLOAT and TARGET_SINGLE_FLOAT conditions on the DFmode resp. SFmode cases. --- gcc/config/rs6000/rs6000.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 7e3e5a6..86324ba 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -11453,9 +11453,9 @@ abi_v4_pass_in_fpr (machine_mode mode, bool named) { if (!TARGET_HARD_FLOAT) return false; - if (mode == DFmode) + if (TARGET_DOUBLE_FLOAT && mode == DFmode) return true; - if (mode == SFmode && named) + if (TARGET_SINGLE_FLOAT && mode == SFmode && named) return true; /* ABI_V4 passes complex IBM long double in 8 gprs. Stupid, but we can't change the ABI now. */ -- 1.8.3.1
Re: [Aarch64] Fix conditional branches with target far away.
Ping! On 28 February 2018 at 16:18, Sameera Deshpande wrote: > On 27 February 2018 at 18:25, Ramana Radhakrishnan > wrote: >> On Wed, Feb 14, 2018 at 8:30 AM, Sameera Deshpande >> wrote: >>> Hi! >>> >>> Please find attached the patch to fix bug in branches with offsets over >>> 1MiB. >>> There has been an attempt to fix this issue in commit >>> 050af05b9761f1979f11c151519e7244d5becd7c >>> >>> However, the far_branch attribute defined in above patch used >>> insn_length - which computes incorrect offset. Hence, eliminated the >>> attribute completely, and computed the offset from insn_addresses >>> instead. >>> >>> Ok for trunk? >>> >>> gcc/Changelog >>> >>> 2018-02-13 Sameera Deshpande >>> * config/aarch64/aarch64.md (far_branch): Remove attribute. >>> Eliminate >>> all the dependencies on the attribute from RTL patterns. >>> >> >> I'm not a maintainer but this looks good to me modulo notes about how >> this was tested. What would be nice is a testcase for the testsuite as >> well as ensuring that the patch has been bootstrapped and regression >> tested. AFAIR, the original patch was put in because match.pd failed >> when bootstrap in another context. >> >> >> regards >> Ramana >> >>> -- >>> - Thanks and regards, >>> Sameera D. > > The patch is tested with GCC testsuite and bootstrapping successfully. > Also tested for spec benchmark. > > -- > - Thanks and regards, > Sameera D. -- - Thanks and regards, Sameera D.
Re: [PATCH] Fix PR84873
On Thu, Mar 15, 2018 at 01:56:16PM +0100, Richard Biener wrote: > The following fixes the C familiy gimplification langhook to not > introduce tree sharing which isn't valid during gimplification. > For the specific case the tree sharing is introduced by > fold_binary_op_with_cond and is reached via convert () eventually > folding something. I've kept folding constants here but for the > rest defer folding to GIMPLE (the gimplifier already folds most > generated stmts). > > Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk and > branches? > > Thanks, > Richard. > > 2018-03-15 Richard Biener > > PR c/84873 > * c-gimplify.c (c_gimplify_expr): Do not fold expressions. > > * c-c++-common/pr84873.c: New testcase. Ok, thanks. Jakub
GW Pharmaceuticals & UC Irvine to Keynote Cannabinoid Therapeutics Symposium
CANNABINOID THERAPEUTICS SYMPOSIUM June 6 - 7, 2018 | Crowne Plaza Redondo Beach & Marina Hotel | Redondo Beach, CA --- SIGN UP TODAY: http://links.infocastevents.mkt8115.com/ctt?kn=5&ms=MzM0ODQwOTES1&r=NjkyMTk1NzM3MTk0S0&b=2&j=MTI0MTgzMjkwMwS2&mt=1&rt=0 --- Get a status report from world-class biomedical researchers on the cutting-edge of cannabis and ECS-based medicine at the Cannabinoid Therapeutics Symposium, including keynote talks from: Daniele Piomelli, PhD, MD (h.c.), UC IRVINE, will address The Future of Cannabis-based Therapeutics: How endocannabinoids work and ways to leverage their protective roles to create safe and effective medicines for pain and stress-related disorders. Roger G. Pertwee, MA, DPhil, DSc, HonFBPhS GW Pharmaceuticals; University of Aberdeen, will speak to potential novel therapeutic uses of phytocannabinoids and of certain synthetic analogues of phytocannabinoids. Our keynote speakers will be joined by other world-class biomedical researchers, clinicians and more, including: Ziva Cooper, PhD: COLUMBIA UNIVERSITY MEDICAL CENTER Ziva Cooper, PhD Associate Professor of Clinical Neurobiology (in Psychology) COLUMBIA UNIVERSITY MEDICAL CENTER Chris Evans, PhD: UCLA Chris Evans, PhD Investigator, Center for Translational Technologies; Director, Brain Research Institute UCLA Barbara A. Brett, PhD: CORADO STATE UNIVERSITY - PUEBLO Barbara A. Brett, PhD Associate Professor, Dept. of Psychology CORADO STATE UNIVERSITY - PUEBLO Bonni Goldstein, MD: CANNA-CENTERS Bonni Goldstein, MD Medical Director CANNA-CENTERS --- SIGN UP TODAY: http://links.infocastevents.mkt8115.com/ctt?kn=9&ms=MzM0ODQwOTES1&r=NjkyMTk1NzM3MTk0S0&b=2&j=MTI0MTgzMjkwMwS2&mt=1&rt=0 - Unsubscribe http://www.pages03.net/informationforecastinc/SubscriptionPreferences/SubPreferences?spMailingID=33484091&spUserID=NjkyMTk1NzM3MTk0S0&spJobID=MTI0MTgzMjkwMwS2&spReportId=MTI0MTgzMjkwMwS2
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 3:08 PM, Jakub Jelinek wrote: > On Thu, Mar 15, 2018 at 07:10:26AM -0700, Steve Kargl wrote: >> On Thu, Mar 15, 2018 at 01:35:23PM +0100, Richard Biener wrote: >> > > >> > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); >> > > extern __typeof(kill) kill __asm__("" "_gfortran_kill"); >> > >> > Why do you need to jump through these hoops anyway? Just do ... >> > >> >> Not sure who the "you" refers to. The easiest >> fix be appending a 4 to kill. I'll do that >> later. > > I think this is even easier, no need to rename anything: > > 2018-03-15 Jakub Jelinek > > PR libgfortran/84880 > * intrinsics/kill.c (kill): Rename to... > (PREFIX (kill)): ... this. Use export_proto_np instead of > export_proto. > > --- libgfortran/intrinsics/kill.c.jj2018-03-14 09:44:57.988975360 +0100 > +++ libgfortran/intrinsics/kill.c 2018-03-15 16:01:02.725668658 +0100 > @@ -51,11 +51,11 @@ kill_sub (GFC_INTEGER_4 pid, GFC_INTEGER > } > iexport(kill_sub); > > -extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > -export_proto(kill); > +extern GFC_INTEGER_4 PREFIX (kill) (GFC_INTEGER_4, GFC_INTEGER_4); > +export_proto_np(PREFIX (kill)); > > GFC_INTEGER_4 > -kill (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) > +PREFIX (kill) (GFC_INTEGER_4 pid, GFC_INTEGER_4 signal) > { >int val; >val = (int)kill (pid, signal); Hi, FYI, both your patches fix the compilation issue. Thanks, bin > > > Jakub
PING^3: [PATCH] i386: Don't generate alias for function return thunk
On Sun, Mar 11, 2018 at 7:39 AM, H.J. Lu wrote: > On Mon, Mar 5, 2018 at 4:17 AM, H.J. Lu wrote: >> On Mon, Feb 26, 2018 at 12:48 PM, H.J. Lu wrote: >>> Function return thunks shouldn't be aliased to indirect branch thunks >>> since indirect branch thunks are placed in COMDAT section and a COMDAT >>> section with indirect branch may not have function return thunk. This >>> patch generates function return thunks directly. >>> >>> Tested on i686 and x86-64. OK for trunk? >>> >>> H.J. >>> --- >>> gcc/ >>> >>> PR target/84574 >>> * config/i386/i386.c (indirect_thunk_needed): Update comments. >>> (indirect_thunk_bnd_needed): Likewise. >>> (indirect_thunks_used): Likewise. >>> (indirect_thunks_bnd_used): Likewise. >>> (indirect_return_needed): New. >>> (indirect_return_bnd_needed): Likewise. >>> (output_indirect_thunk_function): Add a bool argument for >>> function return. >>> (output_indirect_thunk_function): Don't generate alias for >>> function return thunk. >>> (ix86_code_end): Call output_indirect_thunk_function to generate >>> function return thunks. >>> (ix86_output_function_return): Set indirect_return_bnd_needed >>> and indirect_return_needed instead of indirect_thunk_bnd_needed >>> and indirect_thunk_needed. >>> >>> gcc/testsuite/ >>> >>> PR target/84574 >>> * gcc.target/i386/ret-thunk-9.c: Expect __x86_return_thunk >>> label instead of __x86_indirect_thunk label. >> >> PING: >> >> https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01460.html >> > > PING. > PING. -- H.J.
PING^3: [PATCH] x86: Force __x86_indirect_thunk_reg for function call via GOT
On Sun, Mar 11, 2018 at 7:40 AM, H.J. Lu wrote: > On Mon, Mar 5, 2018 at 4:20 AM, H.J. Lu wrote: >> On Tue, Feb 27, 2018 at 11:39 AM, H.J. Lu wrote: >>> For x86 targets, when -fno-plt is used, external functions are called >>> via GOT slot, in 64-bit mode: >>> >>> [bnd] call/jmp *foo@GOTPCREL(%rip) >>> >>> and in 32-bit mode: >>> >>> [bnd] call/jmp *foo@GOT[(%reg)] >>> >>> With -mindirect-branch=, they are converted to, in 64-bit mode: >>> >>> pushq foo@GOTPCREL(%rip) >>> [bnd] jmp __x86_indirect_thunk[_bnd] >>> >>> and in 32-bit mode: >>> >>> pushl foo@GOT[(%reg)] >>> [bnd] jmp __x86_indirect_thunk[_bnd] >>> >>> which were incompatible with CFI. In 64-bit mode, since R11 is a scratch >>> register, we generate: >>> >>> movq foo@GOTPCREL(%rip), %r11 >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 >>> >>> instead. We do it in ix86_output_indirect_branch so that we can use >>> the newly proposed R_X86_64_THUNK_GOTPCRELX relocation: >>> >>> https://groups.google.com/forum/#!topic/x86-64-abi/eED5lzn3_Mg >>> >>> movq foo@OTPCREL_THUNK(%rip), %r11 >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 >>> >>> to load GOT slot into R11. If foo is defined locally, linker can can >>> convert >>> >>> movq foo@GOTPCREL_THUNK(%rip), %reg >>> call/jmp __x86_indirect_thunk_reg >>> >>> to >>> >>> call/jmp foo >>> nop0L(%rax) >>> >>> In 32-bit mode, since all caller-saved registers, EAX, EDX and ECX, may >>> used to function parameters, there is no scratch register available. For >>> -fno-plt -fno-pic -mindirect-branch=, we expand external function call >>> to: >>> >>> movl foo@GOT, %reg >>> [bnd] call/jmp *%reg >>> >>> so that it can be converted to >>> >>> movl foo@GOT, %reg >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]reg >>> >>> in ix86_output_indirect_branch. Since this is performed during RTL >>> expansion, other instructions may be inserted between movl and call/jmp. >>> Linker optimization isn't always possible. >>> >>> Tested on i686 and x86-64. OK for trunk? >>> >>> >>> H.J. >>> --- >>> gcc/ >>> >>> PR target/83970 >>> * config/i386/constraints.md (Bs): Allow GOT_memory_operand >>> for TARGET_LP64 with indirect branch conversion. >>> (Bw): Likewise. >>> * config/i386/i386.c (ix86_expand_call): Handle -fno-plt with >>> -mindirect-branch=. >>> (ix86_nopic_noplt_attribute_p): Likewise. >>> (ix86_output_indirect_branch): In 64-bit mode, convert function >>> call via GOT with R11 as a scratch register using >>> __x86_indirect_thunk_r11. >>> (ix86_output_call_insn): In 64-bit mode, set xasm to NULL when >>> calling ix86_output_indirect_branch with function call via GOT. >>> * config/i386/i386.md (*call_got_thunk): New call pattern for >>> TARGET_LP64 with indirect branch conversion. >>> (*call_value_got_thunk): Likewise. >>> >>> gcc/testsuite/ >>> >>> PR target/83970 >>> * gcc.target/i386/indirect-thunk-5.c: Updated. >>> * gcc.target/i386/indirect-thunk-6.c: Likewise. >>> * gcc.target/i386/indirect-thunk-bnd-3.c: Likewise. >>> * gcc.target/i386/indirect-thunk-bnd-4.c: Likewise. >>> * gcc.target/i386/indirect-thunk-extern-5.c: Likewise. >>> * gcc.target/i386/indirect-thunk-extern-6.c: Likewise. >>> * gcc.target/i386/indirect-thunk-inline-5.c: Likewise. >>> * gcc.target/i386/indirect-thunk-inline-6.c: Likewise. >>> * gcc.target/i386/indirect-thunk-13.c: New test. >>> * gcc.target/i386/indirect-thunk-14.c: Likewise. >>> * gcc.target/i386/indirect-thunk-bnd-5.c: Likewise. >>> * gcc.target/i386/indirect-thunk-bnd-6.c: Likewise. >>> * gcc.target/i386/indirect-thunk-extern-11.c: Likewise. >>> * gcc.target/i386/indirect-thunk-extern-12.c: Likewise. >>> * gcc.target/i386/indirect-thunk-inline-8.c: Likewise. >>> * gcc.target/i386/indirect-thunk-inline-9.c: Likewise. >> >> PING: >> >> https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01527.html >> > > PING. > PING. -- H.J.
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
> On Sun, Mar 11, 2018 at 7:39 AM, H.J. Lu wrote: > > On Mon, Mar 5, 2018 at 4:17 AM, H.J. Lu wrote: > >> On Mon, Feb 26, 2018 at 12:48 PM, H.J. Lu wrote: > >>> Function return thunks shouldn't be aliased to indirect branch thunks > >>> since indirect branch thunks are placed in COMDAT section and a COMDAT > >>> section with indirect branch may not have function return thunk. This > >>> patch generates function return thunks directly. > >>> > >>> Tested on i686 and x86-64. OK for trunk? > >>> > >>> H.J. > >>> --- > >>> gcc/ > >>> > >>> PR target/84574 > >>> * config/i386/i386.c (indirect_thunk_needed): Update comments. > >>> (indirect_thunk_bnd_needed): Likewise. > >>> (indirect_thunks_used): Likewise. > >>> (indirect_thunks_bnd_used): Likewise. > >>> (indirect_return_needed): New. > >>> (indirect_return_bnd_needed): Likewise. > >>> (output_indirect_thunk_function): Add a bool argument for > >>> function return. > >>> (output_indirect_thunk_function): Don't generate alias for > >>> function return thunk. > >>> (ix86_code_end): Call output_indirect_thunk_function to generate > >>> function return thunks. > >>> (ix86_output_function_return): Set indirect_return_bnd_needed > >>> and indirect_return_needed instead of indirect_thunk_bnd_needed > >>> and indirect_thunk_needed. > >>> > >>> gcc/testsuite/ > >>> > >>> PR target/84574 > >>> * gcc.target/i386/ret-thunk-9.c: Expect __x86_return_thunk > >>> label instead of __x86_indirect_thunk label. > >> So problem here is that you may output the comdat without return thunk from one unit and with return thunk with different unit and comdat merging will then render return thunk unreachable? What is the reason for using different names for return and indirect thunks at first place? Honza
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 03:45:47PM +, Bin.Cheng wrote: > FYI, both your patches fix the compilation issue. It isn't just a compilation problem, it really can't work at all. Without the patch, if the function builds, it looks like: 002308b0 <_gfortran_kill>: 2308b0: f3 0f 1e fa endbr64 2308b4: 48 83 ec 08 sub$0x8,%rsp 2308b8: e8 f3 ff ff ff callq 2308b0 <_gfortran_kill> 2308bd: 85 c0 test %eax,%eax 2308bf: 74 07 je 2308c8 <_gfortran_kill+0x18> 2308c1: e8 4a 92 de ff callq 19b10 <__errno_location@plt> 2308c6: 8b 00 mov(%rax),%eax 2308c8: 48 83 c4 08 add$0x8,%rsp 2308cc: c3 retq 2308cd: 0f 1f 00nopl (%rax) i.e. there is endless recursion, it doesn't call the libc kill, but itself. Jakub
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 3:54 PM, Jakub Jelinek wrote: > On Thu, Mar 15, 2018 at 03:45:47PM +, Bin.Cheng wrote: >> FYI, both your patches fix the compilation issue. > > It isn't just a compilation problem, it really can't work at all. > Without the patch, if the function builds, it looks like: Ah, Thanks. I haven't checked generated code when it builds. Thanks, bin > 002308b0 <_gfortran_kill>: > 2308b0: f3 0f 1e fa endbr64 > 2308b4: 48 83 ec 08 sub$0x8,%rsp > 2308b8: e8 f3 ff ff ff callq 2308b0 <_gfortran_kill> > 2308bd: 85 c0 test %eax,%eax > 2308bf: 74 07 je 2308c8 <_gfortran_kill+0x18> > 2308c1: e8 4a 92 de ff callq 19b10 <__errno_location@plt> > 2308c6: 8b 00 mov(%rax),%eax > 2308c8: 48 83 c4 08 add$0x8,%rsp > 2308cc: c3 retq > 2308cd: 0f 1f 00nopl (%rax) > i.e. there is endless recursion, it doesn't call the libc kill, but itself. > > Jakub
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
On Thu, Mar 15, 2018 at 8:51 AM, Jan Hubicka wrote: >> On Sun, Mar 11, 2018 at 7:39 AM, H.J. Lu wrote: >> > On Mon, Mar 5, 2018 at 4:17 AM, H.J. Lu wrote: >> >> On Mon, Feb 26, 2018 at 12:48 PM, H.J. Lu wrote: >> >>> Function return thunks shouldn't be aliased to indirect branch thunks >> >>> since indirect branch thunks are placed in COMDAT section and a COMDAT >> >>> section with indirect branch may not have function return thunk. This >> >>> patch generates function return thunks directly. >> >>> >> >>> Tested on i686 and x86-64. OK for trunk? >> >>> >> >>> H.J. >> >>> --- >> >>> gcc/ >> >>> >> >>> PR target/84574 >> >>> * config/i386/i386.c (indirect_thunk_needed): Update comments. >> >>> (indirect_thunk_bnd_needed): Likewise. >> >>> (indirect_thunks_used): Likewise. >> >>> (indirect_thunks_bnd_used): Likewise. >> >>> (indirect_return_needed): New. >> >>> (indirect_return_bnd_needed): Likewise. >> >>> (output_indirect_thunk_function): Add a bool argument for >> >>> function return. >> >>> (output_indirect_thunk_function): Don't generate alias for >> >>> function return thunk. >> >>> (ix86_code_end): Call output_indirect_thunk_function to generate >> >>> function return thunks. >> >>> (ix86_output_function_return): Set indirect_return_bnd_needed >> >>> and indirect_return_needed instead of indirect_thunk_bnd_needed >> >>> and indirect_thunk_needed. >> >>> >> >>> gcc/testsuite/ >> >>> >> >>> PR target/84574 >> >>> * gcc.target/i386/ret-thunk-9.c: Expect __x86_return_thunk >> >>> label instead of __x86_indirect_thunk label. >> >> > > So problem here is that you may output the comdat without return thunk from > one unit > and with return thunk with different unit and comdat merging will then render > return thunk > unreachable? Yes. return thunk becomes undefined. > What is the reason for using different names for return and indirect thunks > at first place? > These 2 thunks are identical. But one may want to provide an alternate thunk only for indirect branch and leaves return thunk alone. You can't do that if both have the same name. -- H.J.
[committed] Fix testcase for PR c/84852
On Thu, 2018-03-15 at 09:32 +0800, Paul Hua wrote: > Hi: > > The fixits-pr84852-1.c fails on mips64el target. > > FAIL: gcc.dg/fixits-pr84852-1.c (test for excess errors) > FAIL: gcc.dg/fixits-pr84852-1.c dg-regexp 25 not found: > ".*fixits-pr84852.c:-812156810:25:" > > see this patch: > > diff --git a/gcc/testsuite/gcc.dg/fixits-pr84852-1.c > b/gcc/testsuite/gcc.dg/fixits-pr84852-1.c > index ed88434..98087ab 100644 > --- a/gcc/testsuite/gcc.dg/fixits-pr84852-1.c > +++ b/gcc/testsuite/gcc.dg/fixits-pr84852-1.c > @@ -22,4 +22,4 @@ int foo (void) { return strlen(""); } > #endif > > /* We need this, to consume a stray line marker for the bogus > line. */ > -/* { dg-regexp ".*fixits-pr84852.c:-812156810:25:" } */ > +/* { dg-regexp ".*fixits-pr84852-1.c:-812156810:25:" } */ Thanks for catching this. Sorry about the breakage; it affects every target. I had renamed the testcase, and I missed the failure when reviewing the test results. > On Wed, Mar 14, 2018 at 10:10 PM, David Malcolm > wrote: [...] > > Successfully bootstrapped and regression-tested on x86_64-pc-linux- > > gnu; > > adds 14 PASS results to gcc.sum. [...] Reviewing the logs, it turned out to actually be +12 PASS and +2 FAIL. I'm looking at improving my comparison tool [1] to highlight this better (in my defense, was dealing with a snow day; sorry about the glitch). I've committed the fix to trunk as r258559 (and r258561 to credit you). Dave [1] https://github.com/davidmalcolm/jamais-vu gcc/testsuite/ChangeLog: PR c/84852 * gcc.dg/fixits-pr84852-1.c: Fix filename in dg-regexp. --- gcc/testsuite/gcc.dg/fixits-pr84852-1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.dg/fixits-pr84852-1.c b/gcc/testsuite/gcc.dg/fixits-pr84852-1.c index ed88434..98087ab 100644 --- a/gcc/testsuite/gcc.dg/fixits-pr84852-1.c +++ b/gcc/testsuite/gcc.dg/fixits-pr84852-1.c @@ -22,4 +22,4 @@ int foo (void) { return strlen(""); } #endif /* We need this, to consume a stray line marker for the bogus line. */ -/* { dg-regexp ".*fixits-pr84852.c:-812156810:25:" } */ +/* { dg-regexp ".*fixits-pr84852-1.c:-812156810:25:" } */ -- 1.8.5.3
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
> > What is the reason for using different names for return and indirect thunks > > at first place? > > > > These 2 thunks are identical. But one may want to provide an > alternate thunk only for > indirect branch and leaves return thunk alone. You can't do that if > both have the same > name. Hmm, OK, what is the benefit to have two different thunks? It is just safety precaution so we could adjust one without adjusting the other in future? Honza
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
On Thu, Mar 15, 2018 at 9:03 AM, Jan Hubicka wrote: >> > What is the reason for using different names for return and indirect >> > thunks at first place? >> > >> >> These 2 thunks are identical. But one may want to provide an >> alternate thunk only for >> indirect branch and leaves return thunk alone. You can't do that if >> both have the same >> name. > > Hmm, OK, what is the benefit to have two different thunks? It is just > safety precaution so we could adjust one without adjusting the other in > future? > That is correct. -- H.J.
Re: [PATCH] Fortran -- clean up KILL
On Thu, Mar 15, 2018 at 04:08:10PM +0100, Jakub Jelinek wrote: > On Thu, Mar 15, 2018 at 07:10:26AM -0700, Steve Kargl wrote: > > On Thu, Mar 15, 2018 at 01:35:23PM +0100, Richard Biener wrote: > > > > > > > > extern GFC_INTEGER_4 kill (GFC_INTEGER_4, GFC_INTEGER_4); > > > > extern __typeof(kill) kill __asm__("" "_gfortran_kill"); > > > > > > Why do you need to jump through these hoops anyway? Just do ... > > > > > > > Not sure who the "you" refers to. The easiest > > fix be appending a 4 to kill. I'll do that > > later. > > I think this is even easier, no need to rename anything: > > 2018-03-15 Jakub Jelinek > > PR libgfortran/84880 > * intrinsics/kill.c (kill): Rename to... > (PREFIX (kill)): ... this. Use export_proto_np instead of export_proto. > If you haven't committed your patch, it's OK with me. Sorry about the breakage. -- Steve
Re: [PATCH] Fix PR84873
On Thu, Mar 15, 2018 at 1:07 PM, Jakub Jelinek wrote: > On Thu, Mar 15, 2018 at 01:56:16PM +0100, Richard Biener wrote: >> The following fixes the C familiy gimplification langhook to not >> introduce tree sharing which isn't valid during gimplification. >> For the specific case the tree sharing is introduced by >> fold_binary_op_with_cond and is reached via convert () eventually >> folding something. I've kept folding constants here but for the >> rest defer folding to GIMPLE (the gimplifier already folds most >> generated stmts). >> >> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk and >> branches? Hi, FYI, this causes below failure. Failures: gcc.target/aarch64/var_shift_mask_1.c Bisected to: commit 676d61f64d05af5833ddd471cc99229cedbd59b4 Author: rguenth Date: Thu Mar 15 13:10:24 2018 + 2018-03-15 Richard Biener PR c/84873 * c-gimplify.c (c_gimplify_expr): Do not fold expressions. * c-c++-common/pr84873.c: New testcase. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@258556 138bc75d-0d04-0410-961f-82ee72b054a4 I will get more information about the failure. Thanks, bin >> >> Thanks, >> Richard. >> >> 2018-03-15 Richard Biener >> >> PR c/84873 >> * c-gimplify.c (c_gimplify_expr): Do not fold expressions. >> >> * c-c++-common/pr84873.c: New testcase. > > Ok, thanks. > > Jakub
Re: [Aarch64] Fix conditional branches with target far away.
On 15/03/18 15:27, Sameera Deshpande wrote: Ping! On 28 February 2018 at 16:18, Sameera Deshpande wrote: On 27 February 2018 at 18:25, Ramana Radhakrishnan wrote: On Wed, Feb 14, 2018 at 8:30 AM, Sameera Deshpande wrote: Hi! Please find attached the patch to fix bug in branches with offsets over 1MiB. There has been an attempt to fix this issue in commit 050af05b9761f1979f11c151519e7244d5becd7c However, the far_branch attribute defined in above patch used insn_length - which computes incorrect offset. Hence, eliminated the attribute completely, and computed the offset from insn_addresses instead. Ok for trunk? gcc/Changelog 2018-02-13 Sameera Deshpande * config/aarch64/aarch64.md (far_branch): Remove attribute. Eliminate all the dependencies on the attribute from RTL patterns. I'm not a maintainer but this looks good to me modulo notes about how this was tested. What would be nice is a testcase for the testsuite as well as ensuring that the patch has been bootstrapped and regression tested. AFAIR, the original patch was put in because match.pd failed when bootstrap in another context. regards Ramana -- - Thanks and regards, Sameera D. The patch is tested with GCC testsuite and bootstrapping successfully. Also tested for spec benchmark. I am not a maintainer either. I noticed that the range check you do for the offset has a (<= || >=). The "far_branch" however did (< || >=) for a positive value. Was that also part of the incorrect offset calculation? @@ -692,7 +675,11 @@ { if (get_attr_length (insn) =3D=3D 8) { - if (get_attr_far_branch (insn) =3D=3D 1) + long long int offset; + offset =3D INSN_ADDRESSES (INSN_UID (XEXP (operands[2], 0))) + - INSN_ADDRESSES (INSN_UID (insn)); + + if (offset <=3D -1048576 || offset >=3D 1048572) return aarch64_gen_far_branch (operands, 2, "Ltb", "\\t%0, %1, "); else @@ -709,12 +696,7 @@ (if_then_else (and (ge (minus (match_dup 2) (pc)) (const_int -32768)) (lt (minus (match_dup 2) (pc)) (const_int 32764))) (const_int 4) - (const_int 8))) - (set (attr "far_branch") - (if_then_else (and (ge (minus (match_dup 2) (pc)) (const_int -1048576)) - (lt (minus (match_dup 2) (pc)) (const_int 1048572))) - (const_int 0) - (const_int 1)))] + (const_int 8)))] ) Thanks Sudi -- - Thanks and regards, Sameera D.
libgo patch committed: Force LANG=C when looking for compiler version
This libgo patch forces LANG=C when looking for the version of the C compiler. Tested by installing the gcc-locales package and running LANG=de_DE.utf8 go build hello.go Without this change, that fails, as described at GCC PR 84765. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian Index: gcc/go/gofrontend/MERGE === --- gcc/go/gofrontend/MERGE (revision 258392) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -ce28919112dbb234366816ab39ce060ad45e8ca9 +e4464efc767b8dee4f4c18ffaf6c891f7b9deee7 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/go/cmd/go/internal/work/buildid.go === --- libgo/go/cmd/go/internal/work/buildid.go(revision 257914) +++ libgo/go/cmd/go/internal/work/buildid.go(working copy) @@ -234,7 +234,18 @@ func (b *Builder) gccgoToolID(name, lang // compile an empty file on standard input. cmdline := str.StringList(cfg.BuildToolexec, name, "-###", "-x", language, "-c", "-") cmd := exec.Command(cmdline[0], cmdline[1:]...) - cmd.Env = base.EnvForDir(cmd.Dir, os.Environ()) + + // Strip any LANG or LC_ environment variables, and force + // LANG=C, so that we get the untranslated output. + var env []string + for _, e := range os.Environ() { + if !strings.HasPrefix(e, "LANG=") && !strings.HasPrefix(e, "LC_") { + env = append(env, e) + } + } + env = append(env, "LANG=C") + + cmd.Env = base.EnvForDir(cmd.Dir, env) out, err := cmd.CombinedOutput() if err != nil { return "", fmt.Errorf("%s: %v; output: %q", name, err, out)
Re: [RFC Patch], PowerPC memory support pre-gcc9, patch #2
This is patch #2 of my series for improving the PowerPC internal memory support. It assumes patch #1 has been applied. This patch moves the rs6000_move_128bit function from rs6000.c to a new file, rs6000-output.c. The third patch will create a rs6000_move_64bit function and change both 32-bit and 64-bit movdi to call it, instead of having all of the instructions be literals. I will also likely add improvements to setting the reg_addr address masks for offsetable addresses. The fourth patch will like move movdd and movdf to call rs6000_move_64bit as well. I tested this on a little endian power8 system and there were no regressions. 2018-03-14 Michael Meissner * config.gcc (powerpc*-*-*): Add rs6000-output.o to extra_objs. * config/rs6000/t-rs6000 (rs6000-output.o): Add build rule. * config/rs6000/rs6000.c (rs6000_output_move_128bit): Move to rs6000-output.c. (rs6000_move_128bit_ok_p): Likewise. (rs6000_split_128bit_ok_p): Likewise. * config/rs6000/rs6000-output.c (rs6000_output_move_128bit): Likewise. to rs6000-output.c. (rs6000_move_128bit_ok_p): Likewise. (rs6000_split_128bit_ok_p): Likewise. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797 Index: gcc/config.gcc === --- gcc/config.gcc (revision 258531) +++ gcc/config.gcc (working copy) @@ -466,7 +466,7 @@ powerpc*-*-*spe*) ;; powerpc*-*-*) cpu_type=rs6000 - extra_objs="rs6000-string.o rs6000-p8swap.o" + extra_objs="rs6000-string.o rs6000-p8swap.o rs6000-output.o" extra_headers="ppc-asm.h altivec.h htmintrin.h htmxlintrin.h" extra_headers="${extra_headers} bmi2intrin.h bmiintrin.h" extra_headers="${extra_headers} xmmintrin.h mm_malloc.h emmintrin.h" Index: gcc/config/rs6000/t-rs6000 === --- gcc/config/rs6000/t-rs6000 (revision 258530) +++ gcc/config/rs6000/t-rs6000 (working copy) @@ -30,6 +30,10 @@ rs6000-string.o: $(srcdir)/config/rs6000 $(COMPILE) $< $(POSTCOMPILE) +rs6000-output.o: $(srcdir)/config/rs6000/rs6000-output.c + $(COMPILE) $< + $(POSTCOMPILE) + rs6000-p8swap.o: $(srcdir)/config/rs6000/rs6000-p8swap.c $(COMPILE) $< $(POSTCOMPILE) Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 258535) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -20921,205 +20921,6 @@ rs6000_debug_can_change_mode_class (mach return ret; } -/* Return a string to do a move operation of 128 bits of data. */ - -const char * -rs6000_output_move_128bit (rtx operands[]) -{ - rtx dest = operands[0]; - rtx src = operands[1]; - machine_mode mode = GET_MODE (dest); - int dest_regno; - int src_regno; - bool dest_gpr_p, dest_fp_p, dest_vmx_p, dest_vsx_p; - bool src_gpr_p, src_fp_p, src_vmx_p, src_vsx_p; - - if (REG_P (dest)) -{ - dest_regno = REGNO (dest); - dest_gpr_p = INT_REGNO_P (dest_regno); - dest_fp_p = FP_REGNO_P (dest_regno); - dest_vmx_p = ALTIVEC_REGNO_P (dest_regno); - dest_vsx_p = dest_fp_p | dest_vmx_p; -} - else -{ - dest_regno = -1; - dest_gpr_p = dest_fp_p = dest_vmx_p = dest_vsx_p = false; -} - - if (REG_P (src)) -{ - src_regno = REGNO (src); - src_gpr_p = INT_REGNO_P (src_regno); - src_fp_p = FP_REGNO_P (src_regno); - src_vmx_p = ALTIVEC_REGNO_P (src_regno); - src_vsx_p = src_fp_p | src_vmx_p; -} - else -{ - src_regno = -1; - src_gpr_p = src_fp_p = src_vmx_p = src_vsx_p = false; -} - - /* Register moves. */ - if (dest_regno >= 0 && src_regno >= 0) -{ - if (dest_gpr_p) - { - if (src_gpr_p) - return "#"; - - if (TARGET_DIRECT_MOVE_128 && src_vsx_p) - return (WORDS_BIG_ENDIAN - ? "mfvsrd %0,%x1\n\tmfvsrld %L0,%x1" - : "mfvsrd %L0,%x1\n\tmfvsrld %0,%x1"); - - else if (TARGET_VSX && TARGET_DIRECT_MOVE && src_vsx_p) - return "#"; - } - - else if (TARGET_VSX && dest_vsx_p) - { - if (src_vsx_p) - return "xxlor %x0,%x1,%x1"; - - else if (TARGET_DIRECT_MOVE_128 && src_gpr_p) - return (WORDS_BIG_ENDIAN - ? "mtvsrdd %x0,%1,%L1" - : "mtvsrdd %x0,%L1,%1"); - - else if (TARGET_DIRECT_MOVE && src_gpr_p) - return "#"; - } - - else if (TARGET_ALTIVEC && dest_vmx_p && src_vmx_p) - return "vor %0,%1,%1"; - - else if (dest_fp_p && src_fp_p) - return "#"; -} - - /* Loads. */ - else if (dest_regno >= 0 && MEM_P (src)) -{ - if (dest_gpr_p) - { - if (TARGET_QUAD_MEMORY && quad_
Re: libgo patch committed: Force LANG=C when looking for compiler version
On Mär 15 2018, Ian Lance Taylor wrote: > + env = append(env, "LANG=C") You should use LC_ALL=C instead. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
[arm-embedded][PATCH] Add multilib mapping for -mcpu=cortex-r52
Hi, Currently -mcpu=cortex-r52 gets assigned the default multilib due to lack of mapping from -mcpu=cortex-r52 to an -march option. This is inconsistent with -march=armv8-r which gets the thumb/v7-ar multilib. This patch adds the appropriate mapping. ChangeLog entry is as follows: *** gcc/ChangeLog.arm *** 2018-03-15 Thomas Preud'homme * config/arm/t-rmprofile: Add mapping from -mcpu=cortex-r52 to -march=armv7. Testing: -mcpu=cortex-r52 -print-multi-directory prints . (ie. default mutlilib) without the patch with a multilib build but prints the expected thumb/v7-ar with the patch. We've decided to apply this patch to the ARM/embedded-7-branch. Best regards, Thomas
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
On Thu, Mar 15, 2018 at 4:32 AM, Jakub Jelinek wrote: > On Wed, Mar 14, 2018 at 08:55:47PM -0400, Jason Merrill wrote: >> > @@ -3192,16 +3198,70 @@ replace_placeholders (tree exp, tree obj >> > return exp; >> > >> >tree *tp = &exp; >> > - hash_set pset; >> > - replace_placeholders_t data = { obj, false, &pset }; >> > + /* Use exp instead of *(type *)&exp. */ >> > + while (TREE_CODE (exp) == INDIRECT_REF) >> > +{ >> > + tree t = TREE_OPERAND (exp, 0); >> > + STRIP_NOPS (t); >> > + if (TREE_CODE (t) == ADDR_EXPR >> > + && (t = TREE_OPERAND (t, 0)) >> > + && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (exp), >> > + TREE_TYPE (t))) >> > + exp = t; >> > + else >> > + break; >> > +} >> >> What's the motivation for this? > > g++.dg/cpp0x/nsdmi13.C ICEs without that, we have there: > a = A({}); > and build_over_call does: > 8163 else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE > (as_base))) > 8164{ > 8165 arg = cp_build_fold_indirect_ref (arg); > 8166 val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); > 8167 /* Handle NSDMI that refer to the object being initialized. > */ > 8168 replace_placeholders (arg, to); > 8169} > where arg is after the cp_build_fold_indirect_ref > *(struct A &) &TARGET_EXPR }> > This is in MODIFY_EXPR rather than INIT_EXPR and the gimplifier through > gimple_fold_indirect_ref_rhs folds the *(struct A &) & away and so there is > no further temporary and thus cp_gimplify_init_expr isn't called to > replace_placeholders, so if we don't replace it here (with to being the a > VAR_DECL), we don't replace it ever. Ah. That's a problem: the language says there's a temporary, so after the assignment a.p should not point to a. So the existing call to replace_placeholders in build_over_call is wrong. Seems like if the initializer for a TARGET_EXPR involves PLACEHOLDER_EXPR, the gimple_fold_modify_expr_rhs treatment of TARGET_EXPR isn't safe for MODIFY_EXPR, for the same reason that cp_gimplify_expr treats INIT_EXPR and MODIFY_EXPR differently. Jason
Re: [C++ PATCH] Fix up -Wdeprecated (PR c++/84222)
On Thu, Mar 15, 2018 at 4:07 AM, Jakub Jelinek wrote: > On Wed, Mar 14, 2018 at 08:44:48PM -0400, Jason Merrill wrote: >> > --- gcc/cp/decl.c.jj2018-03-14 09:44:55.744974946 +0100 >> > +++ gcc/cp/decl.c 2018-03-14 12:18:08.094012453 +0100 >> > @@ -10448,7 +10448,7 @@ grokdeclarator (const cp_declarator *dec >> > suppress reports of deprecated items. */ >> >if (type && TREE_DEPRECATED (type) >> >&& deprecated_state != DEPRECATED_SUPPRESS) >> > -warn_deprecated_use (type, NULL_TREE); >> > +cp_warn_deprecated_use (type); >> >if (type && TREE_CODE (type) == TYPE_DECL) >> > { >> >typedef_decl = type; >> > @@ -10456,7 +10456,7 @@ grokdeclarator (const cp_declarator *dec >> >if (TREE_DEPRECATED (type) >> > && DECL_ARTIFICIAL (typedef_decl) >> > && deprecated_state != DEPRECATED_SUPPRESS) >> > - warn_deprecated_use (type, NULL_TREE); >> > + cp_warn_deprecated_use (type); >> > } >> >/* No type at all: default to `int', and set DEFAULTED_INT >> > because it was not a user-defined typedef. */ >> > @@ -11271,8 +11271,18 @@ grokdeclarator (const cp_declarator *dec >> > explicitp = 2; >> > } >> > >> > - arg_types = grokparms (declarator->u.function.parameters, >> > - &parms); >> > + tree pushed_scope = NULL_TREE; >> > + if (funcdecl_p >> > + && decl_context != FIELD >> > + && inner_declarator->u.id.qualifying_scope >> > + && CLASS_TYPE_P (inner_declarator->u.id.qualifying_scope)) >> > + pushed_scope >> > + = push_scope (inner_declarator->u.id.qualifying_scope); >> >> Can't we use ctype here? > > Inside of classes ctype is non-NULL, but we don't need to push anything, > current_class_type is already the class we care about. > That's the > tree ctype = current_class_type; > on line 10130. Then we have this > for (id_declarator = declarator; >id_declarator; >id_declarator = id_declarator->declarator) > loop, where for cdk_id at line 10242 it tweaks ctype: > else if (TYPE_P (qualifying_scope)) > { > ctype = qualifying_scope; > if (!MAYBE_CLASS_TYPE_P (ctype)) > { > error ("%q#T is not a class or a namespace", ctype); > ctype = NULL_TREE; > } > and indeed for the cases I care about (out of class method definitions) > ctype is set to non-NULL here. But then at line 10542 it is cleared again: > ctype = NULL_TREE; > > and at 11176: > if (ctype == NULL_TREE > && decl_context == FIELD > && funcdecl_p > && friendp == 0) > ctype = current_class_type; > set only for selected in-class definitions, and then tested and used a couple > of > times. And that is the state we call grokparms with. > Only later at line 11529 it is set again: > if (declarator > && declarator->kind == cdk_id > && declarator->u.id.qualifying_scope > && MAYBE_CLASS_TYPE_P (declarator->u.id.qualifying_scope)) > { > ctype = declarator->u.id.qualifying_scope; > ctype = TYPE_MAIN_VARIANT (ctype); > So, if I were to use some variable without really changing the behavior of > the grokdeclarator massively, it would need to be a copy of ctype saved into > another variable (how it should be named?) above line 10542. Given the > TYPE_MAIN_VARIANT, I guess we should be using TYPE_MAIN_VARIANT somewhere > too. Wow, yeah, the use of ctype in grokdeclarator is pretty bizarre. Your patch is OK. Jason
Re: libgo patch committed: Force LANG=C when looking for compiler version
On Thu, Mar 15, 2018 at 10:10 AM, Andreas Schwab wrote: > On Mär 15 2018, Ian Lance Taylor wrote: > >> + env = append(env, "LANG=C") > > You should use LC_ALL=C instead. Does it really matter? Do you think that this approach won't work? Ian
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
> On Thu, Mar 15, 2018 at 9:03 AM, Jan Hubicka wrote: > >> > What is the reason for using different names for return and indirect > >> > thunks at first place? > >> > > >> > >> These 2 thunks are identical. But one may want to provide an > >> alternate thunk only for > >> indirect branch and leaves return thunk alone. You can't do that if > >> both have the same > >> name. > > > > Hmm, OK, what is the benefit to have two different thunks? It is just > > safety precaution so we could adjust one without adjusting the other in > > future? > > > > That is correct. Hmm, I guess the patch is OK. Things are slightly more flexible this way and duplicating thunk is not terribly expensive. One can always link with non-comdat+ alias. Honza > > > -- > H.J.
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
On Thu, Mar 15, 2018 at 10:47 AM, Jan Hubicka wrote: >> On Thu, Mar 15, 2018 at 9:03 AM, Jan Hubicka wrote: >> >> > What is the reason for using different names for return and indirect >> >> > thunks at first place? >> >> > >> >> >> >> These 2 thunks are identical. But one may want to provide an >> >> alternate thunk only for >> >> indirect branch and leaves return thunk alone. You can't do that if >> >> both have the same >> >> name. >> > >> > Hmm, OK, what is the benefit to have two different thunks? It is just >> > safety precaution so we could adjust one without adjusting the other in >> > future? >> > >> >> That is correct. > > Hmm, I guess the patch is OK. Things are slightly more flexible this way and > duplicating thunk is not terribly expensive. One can always link with > non-comdat+ alias. > That is true. OK to backport to GCC 7 after a few days? Thanks. -- H.J.
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
> On Thu, Mar 15, 2018 at 10:47 AM, Jan Hubicka wrote: > >> On Thu, Mar 15, 2018 at 9:03 AM, Jan Hubicka wrote: > >> >> > What is the reason for using different names for return and indirect > >> >> > thunks at first place? > >> >> > > >> >> > >> >> These 2 thunks are identical. But one may want to provide an > >> >> alternate thunk only for > >> >> indirect branch and leaves return thunk alone. You can't do that if > >> >> both have the same > >> >> name. > >> > > >> > Hmm, OK, what is the benefit to have two different thunks? It is just > >> > safety precaution so we could adjust one without adjusting the other in > >> > future? > >> > > >> > >> That is correct. > > > > Hmm, I guess the patch is OK. Things are slightly more flexible this way and > > duplicating thunk is not terribly expensive. One can always link with > > non-comdat+ alias. > > > > That is true. OK to backport to GCC 7 after a few days? OK. Honza > > Thanks. > > -- > H.J.
Re: libgo patch committed: Force LANG=C when looking for compiler version
On Mär 15 2018, Ian Lance Taylor wrote: > On Thu, Mar 15, 2018 at 10:10 AM, Andreas Schwab > wrote: >> On Mär 15 2018, Ian Lance Taylor wrote: >> >>> + env = append(env, "LANG=C") >> >> You should use LC_ALL=C instead. > > Does it really matter? Do you think that this approach won't work? LANG has lowest priority. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
[PING][PATCH v3 1/14] D: The front-end (DMD) language implementation and license.
On 17 February 2018 at 16:08, Iain Buclaw wrote: > On 25 October 2017 at 03:06, Jeff Law wrote: >> On 10/18/2017 01:33 AM, Iain Buclaw wrote: >>> On 6 October 2017 at 14:51, Ian Lance Taylor wrote: On Fri, Oct 6, 2017 at 1:34 AM, Iain Buclaw wrote: > > Out of curiosity, I did have a look at some of the tops of gofrontend > sources this morning. They are all copyright the Go Authors, and are > licensed as BSD. So I'm not sure if having copyright FSF and > distributing under GPL is strictly required. And from a maintenance > point of view, it would be easier to merge in upstream changes as-is > without some diff/merging tool. The GCC steering committee accepted the gofrontend code under a non-GPL license with the understanding that the master code would live in a separate repository that would be mirrored into the GCC repo (the master repository for gofrontend is currently at https://go.googlesource.com/gofrontend/). Personally I don't see a problem with doing the same for the D frontend. Ian >>> >>> Should I request that maybe Donald from FSF chime in here? I'd rather >>> avoid another stalemate on this. >> Absolutely, though RMS should probably be included on any discussion >> with Donald. I think the FSF needs to chime in and I think the steering >> committee needs to chime in once we've got guidance from the FSF. >> >> The first and most important question that needs to be answered is >> whether or not the FSF would be OK including the DMD bits with the >> license (boost) as-is into GCC. >> >> If that's not acceptable, then we'd have to look at some kind of script >> to fix the copyrights. >> Jeff >> > > > Just touching base here, hope you all had a good New Year. > > So far, I've only had a general "Yes this is fine" from Ted who's > managing the copyright assignments at the FSF. > > His his initial response being: > --- > If the D files are all Boost v.1 and we can get assignments from all > contributors, there is no problem including the files as there are > currently. Boost is compatible with GPLv3 or later since it is > basically a [permissive license][0]. > > [0]: https://directory.fsf.org/wiki/License:Boost1.0 > --- > > And subsequent follow-up: > --- > The questions that remain still are whether there are any unaccounted > for contributors to this (but I don't believe this is the case from > the first pass). We have the assignment for the past and future code > from Digital Mars. The second question, which is outside of my > discretion is deciding whether the Boost license is acceptable. It > seems that it is compatible so it doesn't appear that incompatibility > is a problem, but of course there are still policy considerations. > These are currently being discussed on the mailing-list and I will add > this message to the thread. > --- > > > I have asked for clarity on a few more finer points, but still haven't > heard back after a number of attempts to get an answer back. > > Can we get discussion rolling again on this? > > Since the last message, upstream dmd has switched all copyrights to > "The D Language Foundation", which has been reflected downstream in > gdc. > > So, as a policy consideration from the SC, is it acceptable to have > the following notice at the top of all dfrontend/* sources? > > --- > Copyright (C) 2010-2018 by The D Language Foundation, All Rights Reserved > All Rights Reserved, written by Walter Bright > http://www.digitalmars.com > Distributed under the Boost Software License, Version 1.0. > (See accompanying file LICENSE or copy at > http://www.boost.org/LICENSE_1_0.txt) > --- > > And if no, what should it instead be? > > There are no restrictions on changing the copyright to FSF and license as > GPLv3+ > > Regards > Iain. Tentative ping on this. I would submit a revived patch set, as active development has not stopped. Just would like input on what would be preferential here. Iain.
Re: libgo patch committed: Force LANG=C when looking for compiler version
On Thu, Mar 15, 2018 at 11:24 AM, Andreas Schwab wrote: > On Mär 15 2018, Ian Lance Taylor wrote: > >> On Thu, Mar 15, 2018 at 10:10 AM, Andreas Schwab >> wrote: >>> On Mär 15 2018, Ian Lance Taylor wrote: >>> + env = append(env, "LANG=C") >>> >>> You should use LC_ALL=C instead. >> >> Does it really matter? Do you think that this approach won't work? > > LANG has lowest priority. The code strips out all the other possibly relevant environment variables. Ian
[PATCH] Fix call expansion ICE (PR c++/79085)
Hi! The following testcase ICEs on arm (or other strict alignment targets). The problem is we have a call that returns a TREE_ADDRESSABLE type, and the lhs of the call is some memory that isn't known to be sufficiently aligned. expand_call in that case allocates a temporary, lets the call return into that object and copies it; this can't work for TREE_ADDRESSABLE types, assign_temp ICEs for those, we don't want to create temporaries in that case. This patch just avoids the temporary for TREE_ADDRESSABLE types and returns directly into the provided target memory. Either the compiler just doesn't know whether the target is properly aligned or not and it is properly aligned at runtime, then it will work properly, or it isn't properly aligned at runtime (perhaps compiler can even prove that) and it will be UB, which is user's fault. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-03-15 Jakub Jelinek PR c++/79085 * calls.c (expand_call): For TREE_ADDRESSABLE rettype ignore alignment check and use address of target always. * g++.dg/opt/pr79085.C: New test. --- gcc/calls.c.jj 2018-02-22 12:37:02.641387687 +0100 +++ gcc/calls.c 2018-03-15 15:17:42.596835316 +0100 @@ -3422,9 +3422,14 @@ expand_call (tree exp, rtx target, int i if (CALL_EXPR_RETURN_SLOT_OPT (exp) && target && MEM_P (target) - && !(MEM_ALIGN (target) < TYPE_ALIGN (rettype) -&& targetm.slow_unaligned_access (TYPE_MODE (rettype), - MEM_ALIGN (target + /* If rettype is addressable, we may not create a temporary. + If target is properly aligned at runtime and the compiler + just doesn't know about it, it will work fine, otherwise it + will be UB. */ + && (TREE_ADDRESSABLE (rettype) + || !(MEM_ALIGN (target) < TYPE_ALIGN (rettype) +&& targetm.slow_unaligned_access (TYPE_MODE (rettype), + MEM_ALIGN (target) structure_value_addr = XEXP (target, 0); else { --- gcc/testsuite/g++.dg/opt/pr79085.C.jj 2018-03-15 15:42:05.382927611 +0100 +++ gcc/testsuite/g++.dg/opt/pr79085.C 2018-03-15 15:32:56.626552182 +0100 @@ -0,0 +1,24 @@ +// PR c++/79085 +// { dg-do compile } +// { dg-options "-Os" } +// { dg-additional-options "-mstrict-align" { target { aarch64*-*-* powerpc*-*-linux* powerpc*-*-elf* } } } + +void *operator new (__SIZE_TYPE__, void *p) { return p; } + +struct S +{ + S (); + S (const S &); + ~S (void); + int i; +}; + +S foo (); + +static char buf [sizeof (S) + 1]; + +S * +bar () +{ + return new (buf + 1) S (foo ()); +} Jakub
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
On Thu, Mar 15, 2018 at 1:59 PM, Jakub Jelinek wrote: > On Thu, Mar 15, 2018 at 01:15:53PM -0400, Jason Merrill wrote: >> > g++.dg/cpp0x/nsdmi13.C ICEs without that, we have there: >> > a = A({}); >> > and build_over_call does: >> > 8163 else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE >> > (as_base))) >> > 8164{ >> > 8165 arg = cp_build_fold_indirect_ref (arg); >> > 8166 val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); >> > 8167 /* Handle NSDMI that refer to the object being >> > initialized. */ >> > 8168 replace_placeholders (arg, to); >> > 8169} >> > where arg is after the cp_build_fold_indirect_ref >> > *(struct A &) &TARGET_EXPR }> >> > This is in MODIFY_EXPR rather than INIT_EXPR and the gimplifier through >> > gimple_fold_indirect_ref_rhs folds the *(struct A &) & away and so there is >> > no further temporary and thus cp_gimplify_init_expr isn't called to >> > replace_placeholders, so if we don't replace it here (with to being the a >> > VAR_DECL), we don't replace it ever. >> >> Ah. That's a problem: the language says there's a temporary, so after >> the assignment a.p should not point to a. > > Seems the gimplify.c stuff: > case INDIRECT_REF: > { > /* If we have code like > > *(const A*)(A*)&x > > where the type of "x" is a (possibly cv-qualified variant > of "A"), treat the entire expression as identical to "x". > This kind of code arises in C++ when an object is bound > to a const reference, and if "x" is a TARGET_EXPR we want > to take advantage of the optimization below. */ > ... > has been added in r92539 as part of PR16405 fix. So, do we want to stop > doing that unconditionally if t is a TARGET_EXPR, or for selected kinds of Folding away the INDIRECT_REF is fine. It's the TARGET_EXPR handling that is wrong. > types of TARGET_EXPR, or ask some langhook whether it is ok to do so > (say not ok if find_placeholders (t))? Or contains_placeholder_p? > Though the last one could also affect Ada and could return true even if > the PLACEHOLDER_EXPRs are for some nested TARGET_EXPR in it. The existing test for whether to collapse a TARGET_EXPR is if (init && !VOID_TYPE_P (TREE_TYPE (init))) so we need this test to fail somehow when the constructor contains placeholders, either by adding an additional test (like the ones you mention) or by making the initialization void in a way that other gimplification knows how to handle. >> So the existing call to replace_placeholders in build_over_call is wrong. > > Shall it be just dropped if we tweak the gimplifier somehow? It must be dropped. >> Seems like if the initializer for a TARGET_EXPR involves >> PLACEHOLDER_EXPR, the gimple_fold_modify_expr_rhs treatment of >> TARGET_EXPR isn't safe for MODIFY_EXPR, for the same reason that >> cp_gimplify_expr treats INIT_EXPR and MODIFY_EXPR differently.
[PATCH] Don't RTL DCE REG_CFA_RESTORE noop moves (PR debug/84875)
Hi! The following testcase ICEs on s390x-linux, because we have 2 registers saved in prologue, but only one of them modified in one path and then both restored there; cprop_hardreg propagates the saving register into the REG_CFA_RESTORE insn, making it a noop move (we don't really need to restore it), and then RTL DCE removes the noop move and we ICE during dwarf2 pass, because of a CFI mismatch. The following patch makes sure that such insns are not actually removed, but turned into a USE still holding the note, which is all we need for CFI. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-03-15 Jakub Jelinek PR debug/84875 * dce.c (delete_unmarked_insns): Don't remove frame related noop moves holding REG_CFA_RESTORE notes, instead turn them into a USE. * gcc.dg/pr84875.c: New test. --- gcc/dce.c.jj2018-01-26 12:47:34.255931988 +0100 +++ gcc/dce.c 2018-03-15 14:23:11.553002165 +0100 @@ -569,9 +569,19 @@ delete_unmarked_insns (void) FOR_BB_INSNS_REVERSE_SAFE (bb, insn, next) if (NONDEBUG_INSN_P (insn)) { + rtx turn_into_use = NULL_RTX; + /* Always delete no-op moves. */ if (noop_move_p (insn)) - ; + { + if (RTX_FRAME_RELATED_P (insn)) + turn_into_use + = find_reg_note (insn, REG_CFA_RESTORE, NULL); + if (turn_into_use && REG_P (XEXP (turn_into_use, 0))) + turn_into_use = XEXP (turn_into_use, 0); + else + turn_into_use = NULL_RTX; + } /* Otherwise rely only on the DCE algorithm. */ else if (marked_insn_p (insn)) @@ -611,8 +621,19 @@ delete_unmarked_insns (void) if (CALL_P (insn)) must_clean = true; - /* Now delete the insn. */ - delete_insn_and_edges (insn); + if (turn_into_use) + { + /* Don't remove frame related noop moves if they cary +REG_CFA_RESTORE note, while we don't need to emit any code, +we need it to emit the CFI restore note. */ + PATTERN (insn) + = gen_rtx_USE (GET_MODE (turn_into_use), turn_into_use); + INSN_CODE (insn) = -1; + df_insn_rescan (insn); + } + else + /* Now delete the insn. */ + delete_insn_and_edges (insn); } /* Deleted a pure or const call. */ --- gcc/testsuite/gcc.dg/pr84875.c.jj 2018-03-15 14:28:02.811159065 +0100 +++ gcc/testsuite/gcc.dg/pr84875.c 2018-03-15 14:27:35.473144347 +0100 @@ -0,0 +1,28 @@ +/* PR debug/84875 */ +/* { dg-do compile } */ +/* { dg-options "-Os" } */ +/* { dg-additional-options "-fpie" { target pie } } */ +/* { dg-additional-options "-march=z196" { target s390*-*-* } } */ + +static long *a[100]; +static int b[100]; +long *c; +int d; +void foo (long *); + +void +bar () +{ + long *g = c; + g--; + d = *g; + if (d) +if (b[d] < 8) + { + *(void **)g = a[d]; + a[d] = g; + b[d]++; + return; + } + foo (g); +} Jakub
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
On Thu, Mar 15, 2018 at 01:15:53PM -0400, Jason Merrill wrote: > > g++.dg/cpp0x/nsdmi13.C ICEs without that, we have there: > > a = A({}); > > and build_over_call does: > > 8163 else if (tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE > > (as_base))) > > 8164{ > > 8165 arg = cp_build_fold_indirect_ref (arg); > > 8166 val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); > > 8167 /* Handle NSDMI that refer to the object being > > initialized. */ > > 8168 replace_placeholders (arg, to); > > 8169} > > where arg is after the cp_build_fold_indirect_ref > > *(struct A &) &TARGET_EXPR }> > > This is in MODIFY_EXPR rather than INIT_EXPR and the gimplifier through > > gimple_fold_indirect_ref_rhs folds the *(struct A &) & away and so there is > > no further temporary and thus cp_gimplify_init_expr isn't called to > > replace_placeholders, so if we don't replace it here (with to being the a > > VAR_DECL), we don't replace it ever. > > Ah. That's a problem: the language says there's a temporary, so after > the assignment a.p should not point to a. Seems the gimplify.c stuff: case INDIRECT_REF: { /* If we have code like *(const A*)(A*)&x where the type of "x" is a (possibly cv-qualified variant of "A"), treat the entire expression as identical to "x". This kind of code arises in C++ when an object is bound to a const reference, and if "x" is a TARGET_EXPR we want to take advantage of the optimization below. */ ... has been added in r92539 as part of PR16405 fix. So, do we want to stop doing that unconditionally if t is a TARGET_EXPR, or for selected kinds of types of TARGET_EXPR, or ask some langhook whether it is ok to do so (say not ok if find_placeholders (t))? Or contains_placeholder_p? Though the last one could also affect Ada and could return true even if the PLACEHOLDER_EXPRs are for some nested TARGET_EXPR in it. > So the existing call to replace_placeholders in build_over_call is wrong. Shall it be just dropped if we tweak the gimplifier somehow? > Seems like if the initializer for a TARGET_EXPR involves > PLACEHOLDER_EXPR, the gimple_fold_modify_expr_rhs treatment of > TARGET_EXPR isn't safe for MODIFY_EXPR, for the same reason that > cp_gimplify_expr treats INIT_EXPR and MODIFY_EXPR differently. Jakub
[PATCH] Fix reassoc with -frounding-math or IBM long double (PR tree-optimization/84841)
Hi! If any argument of * is negated, reassoc adds an artificial -1.0 multiplier. The code then relies on folding at least all those artificial multipliers through const_binop. That function can fail if the result is inexact for -frounding-math or for composite modes, and we only try to recursively fold the last constant with previous one. The following patch fixes it by sorting the constants that never cause inexact results (-1.0 and 1.0) last, so we fold at least all those together and at least with one other constant. As I said in the PR, we could also try up to O(n^2) attempts folding each constant with each other if there were any failures in the folding, perhaps bound by some constant. The comment above constant_type says we want to sort the integral constants last, but we actually among all constants sort them first, so the patch fixes that too. As constant_type returns something (at least before this patch) only based on type, I fail to see when constants in the same ops vector could have different kinds. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-03-15 Jakub Jelinek PR tree-optimization/84841 * tree-ssa-reassoc.c (INTEGER_CONST_TYPE): Change to 1 << 4 from 1 << 3. (FLOAT_ONE_CONST_TYPE): Define. (constant_type): Return FLOAT_ONE_CONST_TYPE for -1.0 and 1.0. (sort_by_operand_rank): Put entries with higher constant_type last rather than first to match comments. * gcc.dg/pr84841.c: New test. --- gcc/tree-ssa-reassoc.c.jj 2018-01-03 10:19:53.804533730 +0100 +++ gcc/tree-ssa-reassoc.c 2018-03-15 17:46:08.871748372 +0100 @@ -470,7 +470,8 @@ get_rank (tree e) /* We want integer ones to end up last no matter what, since they are the ones we can do the most with. */ -#define INTEGER_CONST_TYPE 1 << 3 +#define INTEGER_CONST_TYPE 1 << 4 +#define FLOAT_ONE_CONST_TYPE 1 << 3 #define FLOAT_CONST_TYPE 1 << 2 #define OTHER_CONST_TYPE 1 << 1 @@ -482,7 +483,14 @@ constant_type (tree t) if (INTEGRAL_TYPE_P (TREE_TYPE (t))) return INTEGER_CONST_TYPE; else if (SCALAR_FLOAT_TYPE_P (TREE_TYPE (t))) -return FLOAT_CONST_TYPE; +{ + /* Sort -1.0 and 1.0 constants last, while in some cases +const_binop can't optimize some inexact operations, multiplication +by -1.0 or 1.0 can be always merged with others. */ + if (real_onep (t) || real_minus_onep (t)) + return FLOAT_ONE_CONST_TYPE; + return FLOAT_CONST_TYPE; +} else return OTHER_CONST_TYPE; } @@ -504,7 +512,7 @@ sort_by_operand_rank (const void *pa, co if (oea->rank == 0) { if (constant_type (oeb->op) != constant_type (oea->op)) - return constant_type (oeb->op) - constant_type (oea->op); + return constant_type (oea->op) - constant_type (oeb->op); else /* To make sorting result stable, we use unique IDs to determine order. */ --- gcc/testsuite/gcc.dg/pr84841.c.jj 2018-03-15 17:46:40.142765079 +0100 +++ gcc/testsuite/gcc.dg/pr84841.c 2018-03-15 17:25:05.032150810 +0100 @@ -0,0 +1,9 @@ +/* PR tree-optimization/84841 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fassociative-math -frounding-math -fno-signed-zeros -fno-trapping-math -fno-tree-forwprop" } */ + +double +foo (double x) +{ + return -x * 0.1 * 0.1; +} Jakub
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
> On Thu, Mar 15, 2018 at 10:47 AM, Jan Hubicka wrote: > >> On Thu, Mar 15, 2018 at 9:03 AM, Jan Hubicka wrote: > >> >> > What is the reason for using different names for return and indirect > >> >> > thunks at first place? > >> >> > > >> >> > >> >> These 2 thunks are identical. But one may want to provide an > >> >> alternate thunk only for > >> >> indirect branch and leaves return thunk alone. You can't do that if > >> >> both have the same > >> >> name. > >> > > >> > Hmm, OK, what is the benefit to have two different thunks? It is just > >> > safety precaution so we could adjust one without adjusting the other in > >> > future? > >> > > >> > >> That is correct. > > > > Hmm, I guess the patch is OK. Things are slightly more flexible this way and > > duplicating thunk is not terribly expensive. One can always link with > > non-comdat+ alias. > > > > That is true. OK to backport to GCC 7 after a few days? OK. I suppose you are testing return thunks on some real environment, like GCC bootstrap :) Honza > > Thanks. > > -- > H.J.
Re: [PATCH] Fix call expansion ICE (PR c++/79085)
LGTM. On Thu, Mar 15, 2018 at 3:25 PM, Jakub Jelinek wrote: > Hi! > > The following testcase ICEs on arm (or other strict alignment targets). > The problem is we have a call that returns a TREE_ADDRESSABLE type, > and the lhs of the call is some memory that isn't known to be sufficiently > aligned. expand_call in that case allocates a temporary, lets the call > return into that object and copies it; this can't work for TREE_ADDRESSABLE > types, assign_temp ICEs for those, we don't want to create temporaries in > that case. > This patch just avoids the temporary for TREE_ADDRESSABLE types and returns > directly into the provided target memory. > Either the compiler just doesn't know whether the target is properly aligned > or not and it is properly aligned at runtime, then it will work properly, > or it isn't properly aligned at runtime (perhaps compiler can even prove > that) and it will be UB, which is user's fault. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-03-15 Jakub Jelinek > > PR c++/79085 > * calls.c (expand_call): For TREE_ADDRESSABLE rettype ignore alignment > check and use address of target always. > > * g++.dg/opt/pr79085.C: New test. > > --- gcc/calls.c.jj 2018-02-22 12:37:02.641387687 +0100 > +++ gcc/calls.c 2018-03-15 15:17:42.596835316 +0100 > @@ -3422,9 +3422,14 @@ expand_call (tree exp, rtx target, int i > if (CALL_EXPR_RETURN_SLOT_OPT (exp) > && target > && MEM_P (target) > - && !(MEM_ALIGN (target) < TYPE_ALIGN (rettype) > -&& targetm.slow_unaligned_access (TYPE_MODE (rettype), > - MEM_ALIGN (target > + /* If rettype is addressable, we may not create a temporary. > + If target is properly aligned at runtime and the compiler > + just doesn't know about it, it will work fine, otherwise it > + will be UB. */ > + && (TREE_ADDRESSABLE (rettype) > + || !(MEM_ALIGN (target) < TYPE_ALIGN (rettype) > +&& targetm.slow_unaligned_access (TYPE_MODE (rettype), > + MEM_ALIGN (target) > structure_value_addr = XEXP (target, 0); > else > { > --- gcc/testsuite/g++.dg/opt/pr79085.C.jj 2018-03-15 15:42:05.382927611 > +0100 > +++ gcc/testsuite/g++.dg/opt/pr79085.C 2018-03-15 15:32:56.626552182 +0100 > @@ -0,0 +1,24 @@ > +// PR c++/79085 > +// { dg-do compile } > +// { dg-options "-Os" } > +// { dg-additional-options "-mstrict-align" { target { aarch64*-*-* > powerpc*-*-linux* powerpc*-*-elf* } } } > + > +void *operator new (__SIZE_TYPE__, void *p) { return p; } > + > +struct S > +{ > + S (); > + S (const S &); > + ~S (void); > + int i; > +}; > + > +S foo (); > + > +static char buf [sizeof (S) + 1]; > + > +S * > +bar () > +{ > + return new (buf + 1) S (foo ()); > +} > > Jakub
Re: PING^3: [PATCH] i386: Don't generate alias for function return thunk
On Thu, Mar 15, 2018 at 12:54 PM, Jan Hubicka wrote: >> On Thu, Mar 15, 2018 at 10:47 AM, Jan Hubicka wrote: >> >> On Thu, Mar 15, 2018 at 9:03 AM, Jan Hubicka wrote: >> >> >> > What is the reason for using different names for return and indirect >> >> >> > thunks at first place? >> >> >> > >> >> >> >> >> >> These 2 thunks are identical. But one may want to provide an >> >> >> alternate thunk only for >> >> >> indirect branch and leaves return thunk alone. You can't do that if >> >> >> both have the same >> >> >> name. >> >> > >> >> > Hmm, OK, what is the benefit to have two different thunks? It is just >> >> > safety precaution so we could adjust one without adjusting the other in >> >> > future? >> >> > >> >> >> >> That is correct. >> > >> > Hmm, I guess the patch is OK. Things are slightly more flexible this way >> > and >> > duplicating thunk is not terribly expensive. One can always link with >> > non-comdat+ alias. >> > >> >> That is true. OK to backport to GCC 7 after a few days? > OK. I suppose you are testing return thunks on some real environment, like > GCC bootstrap :) Yes. -- H.J.
[PATCH] PR fortran/78741 -- Detect clash of entry and subroutine name
The patch is fairly slef-explanatory. Regression tested on x86_64-*-freebsd. OK to commit? 2018-03-15 Steven G. Kargl PR fortran/78741 * decl.c (get_proc_name): Check for clash of entry name with subroutine name. 2018-03-15 Steven G. Kargl PR fortran/78741 * gfortran.dg/pr78741.f90: New test. -- Steve Index: gcc/fortran/decl.c === --- gcc/fortran/decl.c (revision 258571) +++ gcc/fortran/decl.c (working copy) @@ -804,7 +804,7 @@ cleanup: static bool merge_array_spec (gfc_array_spec *from, gfc_array_spec *to, bool copy) { - int i; + int i, j; if ((from->type == AS_ASSUMED_RANK && to->corank) || (to->type == AS_ASSUMED_RANK && from->corank)) @@ -822,8 +822,14 @@ merge_array_spec (gfc_array_spec *from, gfc_array_spec for (i = 0; i < to->corank; i++) { - to->lower[from->rank + i] = to->lower[i]; - to->upper[from->rank + i] = to->upper[i]; + /* Do not exceed the limits on lower[] and upper[]. gfortran + cleans up elsewhere. */ + j = from->rank + i; + if (j >= GFC_MAX_DIMENSIONS) + break; + + to->lower[j] = to->lower[i]; + to->upper[j] = to->upper[i]; } for (i = 0; i < from->rank; i++) { @@ -846,19 +852,33 @@ merge_array_spec (gfc_array_spec *from, gfc_array_spec for (i = 0; i < from->corank; i++) { + /* Do not exceed the limits on lower[] and upper[]. gfortran + cleans up elsewhere. */ + j = to->rank + i; + if (j >= GFC_MAX_DIMENSIONS) + break; + if (copy) { - to->lower[to->rank + i] = gfc_copy_expr (from->lower[i]); - to->upper[to->rank + i] = gfc_copy_expr (from->upper[i]); + to->lower[j] = gfc_copy_expr (from->lower[i]); + to->upper[j] = gfc_copy_expr (from->upper[i]); } else { - to->lower[to->rank + i] = from->lower[i]; - to->upper[to->rank + i] = from->upper[i]; + to->lower[j] = from->lower[i]; + to->upper[j] = from->upper[i]; } } } + if (to->rank + to->corank >= GFC_MAX_DIMENSIONS) +{ + gfc_error ("Sum of array rank %d and corank %d at %C exceeds maximum " + "allowed dimensions of %d", + to->rank, to->corank, GFC_MAX_DIMENSIONS); + to->corank = GFC_MAX_DIMENSIONS - to->rank; + return false; +} return true; } @@ -1189,8 +1209,13 @@ get_proc_name (const char *name, gfc_symbol **result, accessible names. */ if (sym->attr.flavor != 0 && sym->attr.proc != 0 - && (sym->attr.subroutine || sym->attr.function) + && (sym->attr.subroutine || sym->attr.function || sym->attr.entry) && sym->attr.if_source != IFSRC_UNKNOWN) + gfc_error_now ("Procedure %qs at %C is already defined at %L", + name, &sym->declared_at); + + if (sym->attr.flavor != 0 + && sym->attr.entry && sym->attr.if_source != IFSRC_UNKNOWN) gfc_error_now ("Procedure %qs at %C is already defined at %L", name, &sym->declared_at); Index: gcc/testsuite/gfortran.dg/pr78741.f90 === --- gcc/testsuite/gfortran.dg/pr78741.f90 (nonexistent) +++ gcc/testsuite/gfortran.dg/pr78741.f90 (working copy) @@ -0,0 +1,16 @@ +! { dg-do compile } +! PR fortran/78741 +! Contributed by Gerhard Steinmetz +subroutine s(n, x) + integer :: n + character(n) :: x + character, pointer :: z(:) + x = 'a' + return +entry g(n, x) ! { dg-error "is already defined" } + x = 'b' +contains + subroutine g ! { dg-error "(1)" } + z(1) = x(1:1) + end +end
Re: [PATCH] PR fortran/69395 -- don't exceed max allowed array dimensions
Hi Steve, The attachedi patch detects situations where the sum of an array's rank and corank exceeds the maximum allowed by the Standard. Regression tested on x86_64-*-freebsd. OK for trunk. Thanks for the patch! Regards, Thomas
Re: [PATCH] PR fortran/78741 -- Detect clash of entry and subroutine name
On Thu, Mar 15, 2018 at 01:18:02PM -0700, Steve Kargl wrote: > The patch is fairly slef-explanatory. Well, it would be self-explanatory if I sent the right patch. (Two pathces touch same file). -- Steve Index: gcc/fortran/decl.c === --- gcc/fortran/decl.c (revision 258571) +++ gcc/fortran/decl.c (working copy) @@ -1189,8 +1209,13 @@ get_proc_name (const char *name, gfc_symbol **result, accessible names. */ if (sym->attr.flavor != 0 && sym->attr.proc != 0 - && (sym->attr.subroutine || sym->attr.function) + && (sym->attr.subroutine || sym->attr.function || sym->attr.entry) && sym->attr.if_source != IFSRC_UNKNOWN) + gfc_error_now ("Procedure %qs at %C is already defined at %L", + name, &sym->declared_at); + + if (sym->attr.flavor != 0 + && sym->attr.entry && sym->attr.if_source != IFSRC_UNKNOWN) gfc_error_now ("Procedure %qs at %C is already defined at %L", name, &sym->declared_at); Index: gcc/testsuite/gfortran.dg/pr78741.f90 === --- gcc/testsuite/gfortran.dg/pr78741.f90 (nonexistent) +++ gcc/testsuite/gfortran.dg/pr78741.f90 (working copy) @@ -0,0 +1,16 @@ +! { dg-do compile } +! PR fortran/78741 +! Contributed by Gerhard Steinmetz +subroutine s(n, x) + integer :: n + character(n) :: x + character, pointer :: z(:) + x = 'a' + return +entry g(n, x) ! { dg-error "is already defined" } + x = 'b' +contains + subroutine g ! { dg-error "(1)" } + z(1) = x(1:1) + end +end
Re: PING^3: [PATCH] x86: Force __x86_indirect_thunk_reg for function call via GOT
> On Sun, Mar 11, 2018 at 7:40 AM, H.J. Lu wrote: > > On Mon, Mar 5, 2018 at 4:20 AM, H.J. Lu wrote: > >> On Tue, Feb 27, 2018 at 11:39 AM, H.J. Lu wrote: > >>> For x86 targets, when -fno-plt is used, external functions are called > >>> via GOT slot, in 64-bit mode: > >>> > >>> [bnd] call/jmp *foo@GOTPCREL(%rip) > >>> > >>> and in 32-bit mode: > >>> > >>> [bnd] call/jmp *foo@GOT[(%reg)] > >>> > >>> With -mindirect-branch=, they are converted to, in 64-bit mode: > >>> > >>> pushq foo@GOTPCREL(%rip) > >>> [bnd] jmp __x86_indirect_thunk[_bnd] > >>> > >>> and in 32-bit mode: > >>> > >>> pushl foo@GOT[(%reg)] > >>> [bnd] jmp __x86_indirect_thunk[_bnd] > >>> > >>> which were incompatible with CFI. In 64-bit mode, since R11 is a scratch > >>> register, we generate: > >>> > >>> movq foo@GOTPCREL(%rip), %r11 > >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 > >>> > >>> instead. We do it in ix86_output_indirect_branch so that we can use > >>> the newly proposed R_X86_64_THUNK_GOTPCRELX relocation: > >>> > >>> https://groups.google.com/forum/#!topic/x86-64-abi/eED5lzn3_Mg > >>> > >>> movq foo@OTPCREL_THUNK(%rip), %r11 > >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 > >>> > >>> to load GOT slot into R11. If foo is defined locally, linker can can > >>> convert > >>> > >>> movq foo@GOTPCREL_THUNK(%rip), %reg > >>> call/jmp __x86_indirect_thunk_reg > >>> > >>> to > >>> > >>> call/jmp foo > >>> nop0L(%rax) > >>> > >>> In 32-bit mode, since all caller-saved registers, EAX, EDX and ECX, may > >>> used to function parameters, there is no scratch register available. For > >>> -fno-plt -fno-pic -mindirect-branch=, we expand external function call > >>> to: > >>> > >>> movl foo@GOT, %reg > >>> [bnd] call/jmp *%reg > >>> > >>> so that it can be converted to > >>> > >>> movl foo@GOT, %reg > >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]reg > >>> > >>> in ix86_output_indirect_branch. Since this is performed during RTL > >>> expansion, other instructions may be inserted between movl and call/jmp. > >>> Linker optimization isn't always possible. I suppose we can just combine those into patterns if we want to prevent gcc from interleaving this with other instructions. However since this affects ABI and not only return thunk, did you discuss the changes with LLVM folks as well? I would be nice to not have diverging solutions. Honza
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
On Thu, Mar 15, 2018 at 4:28 PM, Jakub Jelinek wrote: > On Thu, Mar 15, 2018 at 03:33:12PM -0400, Jason Merrill wrote: >> Folding away the INDIRECT_REF is fine. It's the TARGET_EXPR handling >> that is wrong. > > Ah, ok. > >> > types of TARGET_EXPR, or ask some langhook whether it is ok to do so >> > (say not ok if find_placeholders (t))? Or contains_placeholder_p? >> > Though the last one could also affect Ada and could return true even if >> > the PLACEHOLDER_EXPRs are for some nested TARGET_EXPR in it. >> >> The existing test for whether to collapse a TARGET_EXPR is >> >> if (init >> && !VOID_TYPE_P (TREE_TYPE (init))) >> >> so we need this test to fail somehow when the constructor contains >> placeholders, either by adding an additional test (like the ones you >> mention) or by making the initialization void in a way that other >> gimplification knows how to handle. > > So what about this? It uses an unused bit on TARGET_EXPRs rather than > a langhook, but if you prefer a langhook, I can add it. > > Tested so far just with > make check-c++-all RUNTESTFLAGS="dg.exp='pr79937* pr82410.C nsdmi*'" > > 2018-03-15 Jakub Jelinek > > PR c++/79937 > PR c++/82410 > * tree.h (TARGET_EXPR_NO_ELIDE): Define. > * gimplify.c (gimplify_arg, gimplify_modify_expr_rhs): Don't elide > TARGET_EXPRs with TARGET_EXPR_NO_ELIDE flag set. > > * cp-tree.h (CONSTRUCTOR_PLACEHOLDER_BOUNDARY): Define. > (find_placeholder): Declare. > * tree.c (struct replace_placeholders_t): Add exp member. > (replace_placeholders_r): Don't walk into ctors with > CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set, unless they are equal to > d->exp. > (replace_placeholders): Initialize data.exp. > (find_placeholders_r, find_placeholders): New functions. > * typeck2.c (process_init_constructor_record, > process_init_constructor_union): Set CONSTRUCTOR_PLACEHOLDER_BOUNDARY > if adding NSDMI on which find_placeholder returns true. > * call.c (build_over_call): Don't call replace_placeholders here. > * cp-gimplify.c (cp_genericize_r): Set TARGET_EXPR_NO_ELIDE on > TARGET_EXPRs with CONSTRUCTOR_PLACEHOLDER_BOUNDARY set on > TARGET_EXPR_INITIAL. > (cp_fold): Copy over CONSTRUCTOR_PLACEHOLDER_BOUNDARY bit to new > ctor. > > * g++.dg/cpp1y/pr79937-1.C: New test. > * g++.dg/cpp1y/pr79937-2.C: New test. > * g++.dg/cpp1y/pr79937-3.C: New test. > * g++.dg/cpp1y/pr79937-4.C: New test. > * g++.dg/cpp1y/pr82410.C: New test. > > +/* Don't elide the initialization of TARGET_EXPR_SLOT for this TARGET_EXPR. > */ > +#define TARGET_EXPR_NO_ELIDE(NODE) (TARGET_EXPR_CHECK > (NODE)->base.private_flag) This should be specifically on the rhs of a MODIFY_EXPR; it's OK to elide on the rhs of an INIT_EXPR. > @@ -3155,7 +3155,8 @@ gimplify_arg (tree *arg_p, gimple_seq *p > { >test = is_gimple_lvalue, fb = fb_either; >/* Also strip a TARGET_EXPR that would force an extra copy. */ > - if (TREE_CODE (*arg_p) == TARGET_EXPR) > + if (TREE_CODE (*arg_p) == TARGET_EXPR > + && !TARGET_EXPR_NO_ELIDE (*arg_p)) This is also an initialization context, so we don't need to check it here. > @@ -5211,6 +5212,7 @@ gimplify_modify_expr_rhs (tree *expr_p, > tree init = TARGET_EXPR_INITIAL (*from_p); > > if (init > + && !TARGET_EXPR_NO_ELIDE (*from_p) Here should check TREE_CODE (*expr_p). Jason
Re: PING^3: [PATCH] x86: Force __x86_indirect_thunk_reg for function call via GOT
On Thu, Mar 15, 2018 at 1:41 PM, Jan Hubicka wrote: >> On Sun, Mar 11, 2018 at 7:40 AM, H.J. Lu wrote: >> > On Mon, Mar 5, 2018 at 4:20 AM, H.J. Lu wrote: >> >> On Tue, Feb 27, 2018 at 11:39 AM, H.J. Lu wrote: >> >>> For x86 targets, when -fno-plt is used, external functions are called >> >>> via GOT slot, in 64-bit mode: >> >>> >> >>> [bnd] call/jmp *foo@GOTPCREL(%rip) >> >>> >> >>> and in 32-bit mode: >> >>> >> >>> [bnd] call/jmp *foo@GOT[(%reg)] >> >>> >> >>> With -mindirect-branch=, they are converted to, in 64-bit mode: >> >>> >> >>> pushq foo@GOTPCREL(%rip) >> >>> [bnd] jmp __x86_indirect_thunk[_bnd] >> >>> >> >>> and in 32-bit mode: >> >>> >> >>> pushl foo@GOT[(%reg)] >> >>> [bnd] jmp __x86_indirect_thunk[_bnd] >> >>> >> >>> which were incompatible with CFI. In 64-bit mode, since R11 is a scratch >> >>> register, we generate: >> >>> >> >>> movq foo@GOTPCREL(%rip), %r11 >> >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 >> >>> >> >>> instead. We do it in ix86_output_indirect_branch so that we can use >> >>> the newly proposed R_X86_64_THUNK_GOTPCRELX relocation: >> >>> >> >>> https://groups.google.com/forum/#!topic/x86-64-abi/eED5lzn3_Mg >> >>> >> >>> movq foo@OTPCREL_THUNK(%rip), %r11 >> >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]r11 >> >>> >> >>> to load GOT slot into R11. If foo is defined locally, linker can can >> >>> convert >> >>> >> >>> movq foo@GOTPCREL_THUNK(%rip), %reg >> >>> call/jmp __x86_indirect_thunk_reg >> >>> >> >>> to >> >>> >> >>> call/jmp foo >> >>> nop0L(%rax) >> >>> >> >>> In 32-bit mode, since all caller-saved registers, EAX, EDX and ECX, may >> >>> used to function parameters, there is no scratch register available. For >> >>> -fno-plt -fno-pic -mindirect-branch=, we expand external function call >> >>> to: >> >>> >> >>> movl foo@GOT, %reg >> >>> [bnd] call/jmp *%reg >> >>> >> >>> so that it can be converted to >> >>> >> >>> movl foo@GOT, %reg >> >>> [bnd] call/jmp __x86_indirect_thunk_[bnd_]reg >> >>> >> >>> in ix86_output_indirect_branch. Since this is performed during RTL >> >>> expansion, other instructions may be inserted between movl and call/jmp. >> >>> Linker optimization isn't always possible. > > I suppose we can just combine those into patterns if we want to prevent gcc > from I will look into it. > interleaving this with other instructions. However since this affects ABI and > not only return thunk, did you discuss the changes with LLVM folks as well? This doesn't change calling convention. The new R_X86_64_THUNK_GOTPCRELX relocation is an optimization. It can be safely treated as R_X86_64_GOTPCRELX. > I would be nice to not have diverging solutions. > That is why I posted the new relocation to x86-64 psABI group. -- H.J.
Re: PING^3: [PATCH] x86: Force __x86_indirect_thunk_reg for function call via GOT
> >> >>> in ix86_output_indirect_branch. Since this is performed during RTL > >> >>> expansion, other instructions may be inserted between movl and > >> >>> call/jmp. > >> >>> Linker optimization isn't always possible. > > > > I suppose we can just combine those into patterns if we want to prevent gcc > > from > > I will look into it. I suppose we may want that for next stage1. Right now it would be nice to keep patches simple if possible... > > > interleaving this with other instructions. However since this affects ABI > > and > > not only return thunk, did you discuss the changes with LLVM folks as well? > > This doesn't change calling convention. The new R_X86_64_THUNK_GOTPCRELX > relocation is an optimization. It can be safely treated as > R_X86_64_GOTPCRELX. > > > I would be nice to not have diverging solutions. > > > > That is why I posted the new relocation to x86-64 psABI group. I wonder if anyone from LLVM camp is reading. I will take a look at the proposal too. Honza > > -- > H.J.
Re: PING^3: [PATCH] x86: Force __x86_indirect_thunk_reg for function call via GOT
On Thu, Mar 15, 2018 at 2:03 PM, Jan Hubicka wrote: >> >> >>> in ix86_output_indirect_branch. Since this is performed during RTL >> >> >>> expansion, other instructions may be inserted between movl and >> >> >>> call/jmp. >> >> >>> Linker optimization isn't always possible. >> > >> > I suppose we can just combine those into patterns if we want to prevent >> > gcc from >> >> I will look into it. > > I suppose we may want that for next stage1. Right now it would be nice to > keep patches > simple if possible... Sure. >> > interleaving this with other instructions. However since this affects ABI >> > and >> > not only return thunk, did you discuss the changes with LLVM folks as well? >> >> This doesn't change calling convention. The new R_X86_64_THUNK_GOTPCRELX >> relocation is an optimization. It can be safely treated as >> R_X86_64_GOTPCRELX. >> >> > I would be nice to not have diverging solutions. >> > >> >> That is why I posted the new relocation to x86-64 psABI group. > > I wonder if anyone from LLVM camp is reading. I will take a look at the > proposal too. > Quite a few LLVM developers subscribe x86-64 psABI mailing list. -- H.J.
[patch, committed] fix nios2 thinko
Our friends at Altera spotted a bug in the Nios II backend that was causing different code to be generated randomly for functions involving taking the address of a symbol. The root of the problem was that in r255492 I goofed and introduced some uses of INTVAL that were not guarded by CONST_INT_P so they were randomly grabbing contents of a SYMBOL_REF rtx instead. AFAICT the generated code was not functionally incorrect, just different; the bug caused the movsi_internal splitter to fail to fire when it was supposed to, but I think LRA was subsequently generating the %hi/%lo pair via a different code path. The fix is pretty obvious add the missing guard. :-P -Sandra 2018-03-15 Sandra Loosemore gcc/ * config/nios2/nios2.md (movsi_internal): Fix thinko in split predicate. Index: gcc/config/nios2/nios2.md === --- gcc/config/nios2/nios2.md (revision 482298) +++ gcc/config/nios2/nios2.md (working copy) @@ -298,9 +298,10 @@ } "(nios2_large_constant_memory_operand_p (operands[0]) || nios2_large_constant_memory_operand_p (operands[1]) -|| (nios2_large_constant_p (operands[1]) -&& !SMALL_INT_UNSIGNED (INTVAL (operands[1])) - && !UPPER16_INT (INTVAL (operands[1]" +|| (nios2_large_constant_p (operands[1]) +&& !(CONST_INT_P (operands[1]) + && (SMALL_INT_UNSIGNED (INTVAL (operands[1])) + || UPPER16_INT (INTVAL (operands[1]))" [(set (match_dup 0) (match_dup 1))] { if (nios2_large_constant_memory_operand_p (operands[0]))
Re: [RFC Patch], PowerPC memory support pre-gcc9, patch #3
This patch moves the instructions for movdi (both 32-bit and 64-bit) into a separate rs6000_output_move_64bit function. As I'm starting to move more stuff to checking the addr_masks instead of doing a lot of if mode == MODE1 || mode == MODE2, etc. I realized that the mult-register types (complex values, long double using IBM double double, etc.) did not have the offset bits set correctly in reg_addr. I also prevented the Altivec load/stores (that give you the free AND with -16) from being generated for multi-register values. I added a function (rs6000_valid_move_p) that replaces the old is operand[0] a register or is operand[1] a register tests. Right now, it generates the same tests, but I may need to add additional conditions in the future. I have done a full bootstrap and make check on a little endian power8 system with no regressions. The next patch will change the MOVDF and MOVDD patterns to use rs6000_output_move_64bit as well. 2018-03-15 Michael Meissner * config/rs6000/rs6000-protos.h (rs6000_output_move_64bit): Add declaration. (rs6000_valid_move_p): Likewise. * config/rs6000/rs6000-output.c (addr_is_xform_p): New helper function to return if an addresses uses X-form (reg+reg). (reg_is_spr_p): New helper function to determine if a register is a SPR. (rs6000_output_move_64bit): New function to return the proper instruction to do a 64-bit move. * config/rs6000/rs6000.c (rs6000_setup_reg_addr_masks): Rework setting offset addresses to assume multi-register values have the proper offset bits set. Do not enable Altivec & -16 on mult-reigster moves. (rs6000_valid_move_p): New function to validate moves. (reg_offset_addressing_ok_p): Add check if the mode and register class support offstable instructions. * config/rs6000/rs6000.md (movdi_internal32): Move instruction literals to rs6000_otuput_move_64bit. Check move validity with rs6000_move_valid_p. (movdi_internal64): Likewise. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797 Index: gcc/config/rs6000/rs6000-protos.h === --- gcc/config/rs6000/rs6000-protos.h (revision 258535) +++ gcc/config/rs6000/rs6000-protos.h (working copy) @@ -52,6 +52,7 @@ extern rtx rs6000_got_register (rtx); extern rtx find_addr_reg (rtx); extern rtx gen_easy_altivec_constant (rtx); extern const char *output_vec_const_move (rtx *); +extern const char *rs6000_output_move_64bit (rtx *); extern const char *rs6000_output_move_128bit (rtx *); extern bool rs6000_move_128bit_ok_p (rtx []); extern bool rs6000_split_128bit_ok_p (rtx []); @@ -89,6 +90,7 @@ extern bool rs6000_is_valid_2insn_and (r extern void rs6000_emit_2insn_and (machine_mode, rtx *, bool, int); extern int registers_ok_for_quad_peep (rtx, rtx); extern int mems_ok_for_quad_peep (rtx, rtx); +extern bool rs6000_valid_move_p (rtx, rtx); extern bool gpr_or_gpr_p (rtx, rtx); extern bool direct_move_p (rtx, rtx); extern bool quad_address_p (rtx, machine_mode, bool); Index: gcc/config/rs6000/rs6000-output.c === --- gcc/config/rs6000/rs6000-output.c (revision 258538) +++ gcc/config/rs6000/rs6000-output.c (working copy) @@ -47,6 +47,215 @@ #include "tm-constrs.h" +/* Return whether an address is an x-form (reg or reg+reg) address. This is + used when we know the instruction is not a traditional GPR or FPR + load/store, so check to make sure auto increment is not present in the + address. */ +inline static bool +addr_is_xform_p (rtx addr) +{ + gcc_assert (GET_RTX_CLASS (GET_CODE (addr)) != RTX_AUTOINC); + + if (REG_P (addr) || SUBREG_P (addr)) +return true; + + if (GET_CODE (addr) != PLUS) +return false; + + rtx op1 = XEXP (addr, 1); + return REG_P (op1) || SUBREG_P (op1); +} + +/* Return whether a register is a SPR. */ +inline static bool +reg_is_spr_p (rtx reg) +{ + if (!REG_P (reg)) +return false; + + enum reg_class rclass = REGNO_REG_CLASS (REGNO (reg)); + return reg_class_to_reg_type[(int)rclass] == SPR_REG_TYPE; +} + + +/* Return a string to do a move operation of 64 bits of data. */ + +const char * +rs6000_output_move_64bit (rtx operands[]) +{ + rtx dest = operands[0]; + rtx src = operands[1]; + machine_mode mode = GET_MODE (dest); + int dest_regno; + int src_regno; + bool dest_gpr_p, dest_fp_p, dest_vmx_p, dest_vsx_p; + bool src_gpr_p, src_fp_p, src_vmx_p, src_vsx_p; + + if (REG_P (dest) || SUBREG_P (dest)) +{ + dest_regno = regno_or_subregno (dest); + dest_gpr_p = INT_REGNO_P (dest_regno); + dest_fp_p = FP_REGNO_P (dest_regno); + dest_vmx_p = ALTIVEC_REGNO_P (dest_regno); + dest_vsx_p = dest_fp_p | dest_vmx_p; +} + else +{ +
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
On Thu, Mar 15, 2018 at 04:50:57PM -0400, Jason Merrill wrote: > > +/* Don't elide the initialization of TARGET_EXPR_SLOT for this > > TARGET_EXPR. */ > > +#define TARGET_EXPR_NO_ELIDE(NODE) (TARGET_EXPR_CHECK > > (NODE)->base.private_flag) > > This should be specifically on the rhs of a MODIFY_EXPR; it's OK to > elide on the rhs of an INIT_EXPR. > > > @@ -3155,7 +3155,8 @@ gimplify_arg (tree *arg_p, gimple_seq *p > > { > >test = is_gimple_lvalue, fb = fb_either; > >/* Also strip a TARGET_EXPR that would force an extra copy. */ > > - if (TREE_CODE (*arg_p) == TARGET_EXPR) > > + if (TREE_CODE (*arg_p) == TARGET_EXPR > > + && !TARGET_EXPR_NO_ELIDE (*arg_p)) > > This is also an initialization context, so we don't need to check it here. Ah, ok, changed in the patch. > > @@ -5211,6 +5212,7 @@ gimplify_modify_expr_rhs (tree *expr_p, > > tree init = TARGET_EXPR_INITIAL (*from_p); > > > > if (init > > + && !TARGET_EXPR_NO_ELIDE (*from_p) > > Here should check TREE_CODE (*expr_p). The following (except the *t = unshare_expr (x) change) has successfully bootstrapped/regtested on x86_64-linux and i686-linux (without the unshare_expr it regressed g++.dg/cpp0x/pr83556.C, but with that change it succeeds; I'm afraid right now there is no easy way to determine if we need to unshare or not, if replace_placeholders is called before unshare_body at the start of gimplification, we don't need to unshare, but during gimplification we have to). Ok for trunk? 2018-03-15 Jakub Jelinek PR c++/79937 PR c++/82410 * tree.h (TARGET_EXPR_NO_ELIDE): Define. * gimplify.c (gimplify_modify_expr_rhs): Don't elide TARGET_EXPRs with TARGET_EXPR_NO_ELIDE flag set unless *expr_p is INIT_EXPR. * cp-tree.h (CONSTRUCTOR_PLACEHOLDER_BOUNDARY): Define. (find_placeholder): Declare. * tree.c (struct replace_placeholders_t): Add exp member. (replace_placeholders_r): Don't walk into ctors with CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set, unless they are equal to d->exp. Replace PLACEHOLDER_EXPR with unshare_expr (x) rather than x. (replace_placeholders): Initialize data.exp. (find_placeholders_r, find_placeholders): New functions. * typeck2.c (process_init_constructor_record, process_init_constructor_union): Set CONSTRUCTOR_PLACEHOLDER_BOUNDARY if adding NSDMI on which find_placeholder returns true. * call.c (build_over_call): Don't call replace_placeholders here. * cp-gimplify.c (cp_genericize_r): Set TARGET_EXPR_NO_ELIDE on TARGET_EXPRs with CONSTRUCTOR_PLACEHOLDER_BOUNDARY set on TARGET_EXPR_INITIAL. (cp_fold): Copy over CONSTRUCTOR_PLACEHOLDER_BOUNDARY bit to new ctor. * g++.dg/cpp1y/pr79937-1.C: New test. * g++.dg/cpp1y/pr79937-2.C: New test. * g++.dg/cpp1y/pr79937-3.C: New test. * g++.dg/cpp1y/pr79937-4.C: New test. * g++.dg/cpp1y/pr82410.C: New test. --- gcc/tree.h.jj 2018-02-22 12:37:02.566387732 +0100 +++ gcc/tree.h 2018-03-15 20:42:57.968858551 +0100 @@ -1197,6 +1197,9 @@ extern tree maybe_wrap_with_location (tr #define TARGET_EXPR_SLOT(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 0) #define TARGET_EXPR_INITIAL(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 1) #define TARGET_EXPR_CLEANUP(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 2) +/* Don't elide the initialization of TARGET_EXPR_SLOT for this TARGET_EXPR + on rhs of MODIFY_EXPR. */ +#define TARGET_EXPR_NO_ELIDE(NODE) (TARGET_EXPR_CHECK (NODE)->base.private_flag) /* DECL_EXPR accessor. This gives access to the DECL associated with the given declaration statement. */ --- gcc/gimplify.c.jj 2018-01-18 21:28:49.743301440 +0100 +++ gcc/gimplify.c 2018-03-15 21:06:15.290828320 +0100 @@ -5211,6 +5211,8 @@ gimplify_modify_expr_rhs (tree *expr_p, tree init = TARGET_EXPR_INITIAL (*from_p); if (init + && (TREE_CODE (*expr_p) != MODIFY_EXPR + || !TARGET_EXPR_NO_ELIDE (*from_p)) && !VOID_TYPE_P (TREE_TYPE (init))) { *from_p = init; --- gcc/cp/cp-tree.h.jj 2018-03-15 18:44:21.646300907 +0100 +++ gcc/cp/cp-tree.h2018-03-15 20:40:49.280818226 +0100 @@ -425,6 +425,7 @@ extern GTY(()) tree cp_global_trees[CPTI DECL_VTABLE_OR_VTT_P (in VAR_DECL) FUNCTION_RVALUE_QUALIFIED (in FUNCTION_TYPE, METHOD_TYPE) CALL_EXPR_REVERSE_ARGS (in CALL_EXPR, AGGR_INIT_EXPR) + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (in CONSTRUCTOR) 6: IDENTIFIER_REPO_CHOSEN (in IDENTIFIER_NODE) DECL_CONSTRUCTION_VTABLE_P (in VAR_DECL) TYPE_MARKED_P (in _TYPE) @@ -4144,6 +4145,12 @@ more_aggr_init_expr_args_p (const aggr_i #define CONSTRUCTOR_C99_COMPOUND_LITERAL(NODE) \ (TREE_LANG_FLAG_3 (CONSTRUCTOR_CHECK (NODE))) +/* True if this CONSTRUCTOR co
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
On Thu, Mar 15, 2018 at 03:33:12PM -0400, Jason Merrill wrote: > Folding away the INDIRECT_REF is fine. It's the TARGET_EXPR handling > that is wrong. Ah, ok. > > types of TARGET_EXPR, or ask some langhook whether it is ok to do so > > (say not ok if find_placeholders (t))? Or contains_placeholder_p? > > Though the last one could also affect Ada and could return true even if > > the PLACEHOLDER_EXPRs are for some nested TARGET_EXPR in it. > > The existing test for whether to collapse a TARGET_EXPR is > > if (init > && !VOID_TYPE_P (TREE_TYPE (init))) > > so we need this test to fail somehow when the constructor contains > placeholders, either by adding an additional test (like the ones you > mention) or by making the initialization void in a way that other > gimplification knows how to handle. So what about this? It uses an unused bit on TARGET_EXPRs rather than a langhook, but if you prefer a langhook, I can add it. Tested so far just with make check-c++-all RUNTESTFLAGS="dg.exp='pr79937* pr82410.C nsdmi*'" 2018-03-15 Jakub Jelinek PR c++/79937 PR c++/82410 * tree.h (TARGET_EXPR_NO_ELIDE): Define. * gimplify.c (gimplify_arg, gimplify_modify_expr_rhs): Don't elide TARGET_EXPRs with TARGET_EXPR_NO_ELIDE flag set. * cp-tree.h (CONSTRUCTOR_PLACEHOLDER_BOUNDARY): Define. (find_placeholder): Declare. * tree.c (struct replace_placeholders_t): Add exp member. (replace_placeholders_r): Don't walk into ctors with CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set, unless they are equal to d->exp. (replace_placeholders): Initialize data.exp. (find_placeholders_r, find_placeholders): New functions. * typeck2.c (process_init_constructor_record, process_init_constructor_union): Set CONSTRUCTOR_PLACEHOLDER_BOUNDARY if adding NSDMI on which find_placeholder returns true. * call.c (build_over_call): Don't call replace_placeholders here. * cp-gimplify.c (cp_genericize_r): Set TARGET_EXPR_NO_ELIDE on TARGET_EXPRs with CONSTRUCTOR_PLACEHOLDER_BOUNDARY set on TARGET_EXPR_INITIAL. (cp_fold): Copy over CONSTRUCTOR_PLACEHOLDER_BOUNDARY bit to new ctor. * g++.dg/cpp1y/pr79937-1.C: New test. * g++.dg/cpp1y/pr79937-2.C: New test. * g++.dg/cpp1y/pr79937-3.C: New test. * g++.dg/cpp1y/pr79937-4.C: New test. * g++.dg/cpp1y/pr82410.C: New test. --- gcc/tree.h.jj 2018-02-22 12:37:02.566387732 +0100 +++ gcc/tree.h 2018-03-15 20:42:57.968858551 +0100 @@ -1197,6 +1197,8 @@ extern tree maybe_wrap_with_location (tr #define TARGET_EXPR_SLOT(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 0) #define TARGET_EXPR_INITIAL(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 1) #define TARGET_EXPR_CLEANUP(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 2) +/* Don't elide the initialization of TARGET_EXPR_SLOT for this TARGET_EXPR. */ +#define TARGET_EXPR_NO_ELIDE(NODE) (TARGET_EXPR_CHECK (NODE)->base.private_flag) /* DECL_EXPR accessor. This gives access to the DECL associated with the given declaration statement. */ --- gcc/gimplify.c.jj 2018-01-18 21:28:49.743301440 +0100 +++ gcc/gimplify.c 2018-03-15 21:06:15.290828320 +0100 @@ -3155,7 +3155,8 @@ gimplify_arg (tree *arg_p, gimple_seq *p { test = is_gimple_lvalue, fb = fb_either; /* Also strip a TARGET_EXPR that would force an extra copy. */ - if (TREE_CODE (*arg_p) == TARGET_EXPR) + if (TREE_CODE (*arg_p) == TARGET_EXPR + && !TARGET_EXPR_NO_ELIDE (*arg_p)) { tree init = TARGET_EXPR_INITIAL (*arg_p); if (init @@ -5211,6 +5212,7 @@ gimplify_modify_expr_rhs (tree *expr_p, tree init = TARGET_EXPR_INITIAL (*from_p); if (init + && !TARGET_EXPR_NO_ELIDE (*from_p) && !VOID_TYPE_P (TREE_TYPE (init))) { *from_p = init; --- gcc/cp/cp-tree.h.jj 2018-03-15 18:44:21.646300907 +0100 +++ gcc/cp/cp-tree.h2018-03-15 20:40:49.280818226 +0100 @@ -425,6 +425,7 @@ extern GTY(()) tree cp_global_trees[CPTI DECL_VTABLE_OR_VTT_P (in VAR_DECL) FUNCTION_RVALUE_QUALIFIED (in FUNCTION_TYPE, METHOD_TYPE) CALL_EXPR_REVERSE_ARGS (in CALL_EXPR, AGGR_INIT_EXPR) + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (in CONSTRUCTOR) 6: IDENTIFIER_REPO_CHOSEN (in IDENTIFIER_NODE) DECL_CONSTRUCTION_VTABLE_P (in VAR_DECL) TYPE_MARKED_P (in _TYPE) @@ -4144,6 +4145,12 @@ more_aggr_init_expr_args_p (const aggr_i #define CONSTRUCTOR_C99_COMPOUND_LITERAL(NODE) \ (TREE_LANG_FLAG_3 (CONSTRUCTOR_CHECK (NODE))) +/* True if this CONSTRUCTOR contains PLACEHOLDER_EXPRs referencing the + CONSTRUCTOR's type not nested inside another CONSTRUCTOR marked with + CONSTRUCTOR_PLACEHOLDER_BOUNDARY. */ +#define CONSTRUCTOR_PLACEHOLDER_BOUNDARY(NODE) \ + (TREE_LANG_FLAG_5 (CONSTRUCTOR_CHECK
Re: [PATCH] PR fortran/69395 -- don't exceed max allowed array dimensions
On 03/14/2018 06:23 PM, Steve Kargl wrote: The attachedi patch detects situations where the sum of an array's rank and corank exceeds the maximum allowed by the Standard. Regression tested on x86_64-*-freebsd. 2018-03-14 Steven G. Kargl PR fortran/69395 * decl.c (merge_array_spec): Limit the merging to maximum allowed dimensions, and issue error message if limit is exceeded. 2018-03-14 Steven G. Kargl PR fortran/69395 * gfortran.dg/pr69395.f90 This looks OK. Thanks, Jerry
Re: [C++ PATCH] Fix PLACEHOLDER_EXPR handling (PR c++/79937, PR c++/82410)
On Thu, Mar 15, 2018 at 7:33 PM, Jakub Jelinek wrote: > On Thu, Mar 15, 2018 at 04:50:57PM -0400, Jason Merrill wrote: >> > +/* Don't elide the initialization of TARGET_EXPR_SLOT for this >> > TARGET_EXPR. */ >> > +#define TARGET_EXPR_NO_ELIDE(NODE) (TARGET_EXPR_CHECK >> > (NODE)->base.private_flag) >> >> This should be specifically on the rhs of a MODIFY_EXPR; it's OK to >> elide on the rhs of an INIT_EXPR. >> >> > @@ -3155,7 +3155,8 @@ gimplify_arg (tree *arg_p, gimple_seq *p >> > { >> >test = is_gimple_lvalue, fb = fb_either; >> >/* Also strip a TARGET_EXPR that would force an extra copy. */ >> > - if (TREE_CODE (*arg_p) == TARGET_EXPR) >> > + if (TREE_CODE (*arg_p) == TARGET_EXPR >> > + && !TARGET_EXPR_NO_ELIDE (*arg_p)) >> >> This is also an initialization context, so we don't need to check it here. > > Ah, ok, changed in the patch. > >> > @@ -5211,6 +5212,7 @@ gimplify_modify_expr_rhs (tree *expr_p, >> > tree init = TARGET_EXPR_INITIAL (*from_p); >> > >> > if (init >> > + && !TARGET_EXPR_NO_ELIDE (*from_p) >> >> Here should check TREE_CODE (*expr_p). > > The following (except the *t = unshare_expr (x) change) has successfully > bootstrapped/regtested on x86_64-linux and i686-linux (without the > unshare_expr it regressed g++.dg/cpp0x/pr83556.C, but with that change it > succeeds; I'm afraid right now there is no easy way to determine if we > need to unshare or not, if replace_placeholders is called before > unshare_body at the start of gimplification, we don't need to unshare, but > during gimplification we have to). Ok for trunk? > 2018-03-15 Jakub Jelinek > > PR c++/79937 > PR c++/82410 > * tree.h (TARGET_EXPR_NO_ELIDE): Define. > * gimplify.c (gimplify_modify_expr_rhs): Don't elide TARGET_EXPRs with > TARGET_EXPR_NO_ELIDE flag set unless *expr_p is INIT_EXPR. > > * cp-tree.h (CONSTRUCTOR_PLACEHOLDER_BOUNDARY): Define. > (find_placeholder): Declare. > * tree.c (struct replace_placeholders_t): Add exp member. > (replace_placeholders_r): Don't walk into ctors with > CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set, unless they are equal to > d->exp. Replace PLACEHOLDER_EXPR with unshare_expr (x) rather than x. > (replace_placeholders): Initialize data.exp. > (find_placeholders_r, find_placeholders): New functions. > * typeck2.c (process_init_constructor_record, > process_init_constructor_union): Set CONSTRUCTOR_PLACEHOLDER_BOUNDARY > if adding NSDMI on which find_placeholder returns true. > * call.c (build_over_call): Don't call replace_placeholders here. > * cp-gimplify.c (cp_genericize_r): Set TARGET_EXPR_NO_ELIDE on > TARGET_EXPRs with CONSTRUCTOR_PLACEHOLDER_BOUNDARY set on > TARGET_EXPR_INITIAL. > (cp_fold): Copy over CONSTRUCTOR_PLACEHOLDER_BOUNDARY bit to new > ctor. > > * g++.dg/cpp1y/pr79937-1.C: New test. > * g++.dg/cpp1y/pr79937-2.C: New test. > * g++.dg/cpp1y/pr79937-3.C: New test. > * g++.dg/cpp1y/pr79937-4.C: New test. > * g++.dg/cpp1y/pr82410.C: New test. > > --- gcc/tree.h.jj 2018-02-22 12:37:02.566387732 +0100 > +++ gcc/tree.h 2018-03-15 20:42:57.968858551 +0100 > @@ -1197,6 +1197,9 @@ extern tree maybe_wrap_with_location (tr > #define TARGET_EXPR_SLOT(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 0) > #define TARGET_EXPR_INITIAL(NODE) TREE_OPERAND_CHECK_CODE (NODE, > TARGET_EXPR, 1) > #define TARGET_EXPR_CLEANUP(NODE) TREE_OPERAND_CHECK_CODE (NODE, > TARGET_EXPR, 2) > +/* Don't elide the initialization of TARGET_EXPR_SLOT for this TARGET_EXPR > + on rhs of MODIFY_EXPR. */ > +#define TARGET_EXPR_NO_ELIDE(NODE) (TARGET_EXPR_CHECK > (NODE)->base.private_flag) > > /* DECL_EXPR accessor. This gives access to the DECL associated with > the given declaration statement. */ > --- gcc/gimplify.c.jj 2018-01-18 21:28:49.743301440 +0100 > +++ gcc/gimplify.c 2018-03-15 21:06:15.290828320 +0100 > @@ -5211,6 +5211,8 @@ gimplify_modify_expr_rhs (tree *expr_p, > tree init = TARGET_EXPR_INITIAL (*from_p); > > if (init > + && (TREE_CODE (*expr_p) != MODIFY_EXPR > + || !TARGET_EXPR_NO_ELIDE (*from_p)) > && !VOID_TYPE_P (TREE_TYPE (init))) > { > *from_p = init; > --- gcc/cp/cp-tree.h.jj 2018-03-15 18:44:21.646300907 +0100 > +++ gcc/cp/cp-tree.h2018-03-15 20:40:49.280818226 +0100 > @@ -425,6 +425,7 @@ extern GTY(()) tree cp_global_trees[CPTI >DECL_VTABLE_OR_VTT_P (in VAR_DECL) >FUNCTION_RVALUE_QUALIFIED (in FUNCTION_TYPE, METHOD_TYPE) >CALL_EXPR_REVERSE_ARGS (in CALL_EXPR, AGGR_INIT_EXPR) > + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (in CONSTRUCTOR) > 6: IDENTIFIER_REPO_CHOSEN (in IDENTIFIER_NODE) >DECL_CONSTRUCTION_VTABLE_P (in VAR_DECL) >
Re: [C++ PATCH] Fix ICE in reshape_init_class (PR c++/84874)
OK. On Thu, Mar 15, 2018 at 7:29 PM, Jakub Jelinek wrote: > Hi! > > As the testcase shows, even when we've already reshaped the initializer, if > it uses designated initializers and skips using those over others, we can > have field != d->cur->index. The following patch handles that case. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-03-15 Jakub Jelinek > > PR c++/84874 > * decl.c (reshape_init_class): Don't assert d->cur->index == field > if d->cur->index is a FIELD_DECL, instead set field to d->cur->index. > > * g++.dg/cpp2a/desig7.C: New test. > > --- gcc/cp/decl.c.jj2018-03-15 18:44:21.662300888 +0100 > +++ gcc/cp/decl.c 2018-03-15 19:55:54.831039431 +0100 > @@ -5885,8 +5885,17 @@ reshape_init_class (tree type, reshape_i > return error_mark_node; > > if (TREE_CODE (d->cur->index) == FIELD_DECL) > - /* We already reshaped this. */ > - gcc_assert (d->cur->index == field); > + { > + /* We already reshaped this. */ > + if (field != d->cur->index) > + { > + tree id = DECL_NAME (d->cur->index); > + gcc_assert (id); > + gcc_checking_assert (d->cur->index > + == get_class_binding (type, id, > false)); > + field = d->cur->index; > + } > + } > else if (TREE_CODE (d->cur->index) == IDENTIFIER_NODE) > field = get_class_binding (type, d->cur->index, false); > else > --- gcc/testsuite/g++.dg/cpp2a/desig7.C.jj 2018-03-15 19:54:47.060017400 > +0100 > +++ gcc/testsuite/g++.dg/cpp2a/desig7.C 2018-03-15 19:54:47.060017400 +0100 > @@ -0,0 +1,31 @@ > +// PR c++/84874 > +// { dg-do run { target c++11 } } > +// { dg-options "" } > + > +struct A { int a, b; }; > +struct B { A d; }; > + > +void > +foo (B *x) > +{ > + *x = { .d = { .b = 5 } }; > +} > + > +void > +bar (A *x) > +{ > + *x = { .b = 6 }; > +} > + > +int > +main () > +{ > + B b = { { 2, 3 } }; > + foo (&b); > + if (b.d.a != 0 || b.d.b != 5) > +__builtin_abort (); > + b.d.a = 8; > + bar (&b.d); > + if (b.d.a != 0 || b.d.b != 6) > +__builtin_abort (); > +} > > Jakub
[PATCH][committed][Fortran] Add a few new tests for -fdec-static and -fdec-structure
These are minor testsuite additions to the gfortran testsuite from engineers at Codethink (Mark Doffman and Jim MacArthur). I'm working with Codethink to help identify what, if any, of their work is still potentially relevant for our Fortran implementation. These tests seem like a no-brainer, particularly the tests for errors on ill-formed code. There's 3 new tests for -fdec-static: automatic_1.f90 is a simple execution test automatic_repeat.f90 verifies that the front-end gives an error if AUTOMATIC appears more than once for a variable. automatic_save.f90 verifies that we give an error if we have SAVE and AUTOMATIC for a variable. And there's one new test for -fdec-structure. vax_structure_1.f90 verifies that we get an error if we try to compare fields within a structure that are of different types. I'm taking the liberty of installing these tests onto the trunk. But just to be clear, I wouldn't take such liberties with the front-end/runtime itself. Tested on x86_64-linux-gnu. Installing on the trunk. Jeff
Re: [PATCH] PR gcc/68256 Defining TARGET_USE_CONSTANT_BLOCKS_P causes go bootstrap failure on aarch64.
On 03/15/2018 07:07 AM, Bin.Cheng wrote: > On Fri, Feb 16, 2018 at 5:18 PM, wrote: >> From: Vladimir Mezentsev >> >> Ramana Radhakrishnan made a workaround in gcc/config/aarch64/aarch64.c to >> resolve >> bootstrap comparison failure (2015-11-10, commit >> bc443a71dafa2e707bae4b2fa74f83b05dea37ab). >> The real bug is in gcc/varasm.c. >> hash_section() returns an unstable value. >> As result, two blocks are created in get_block_for_section() for one unnamed >> section. >> A list of objects in these blocks depends on the -gtoggle option. >> I removed Ramana's workaround in gcc/config/aarch64/aarch64.c and >> I fixed hash_section() in gcc/varasm.c >> >> Bootstrapped on aarch64-unknown-linux-gnu including (c,c++ and go). >> Testing finished ok. > Hi Vladimir, > Thanks for fixing the long standing issue, but this change causes below > failure, > could you have a look? Thanks I did not see this failure. I cannot tell why. I use one CFARM machine (gcc116) and when my fix was approved I clean up my directories on this machine (including binaries and log files). ramana.radhakrish...@arm.com implemented (2015-11-06, commit cd4fcdb8ff16ec2dad56f9e736ac4552f8f52001) a new feature ('Switch constant pools to separate rodata sections'). An additional fix is needed for this feature. The test below demonstrates problem: % cat t.c extern void abort(void); typedef int vtype; static vtype Wv12 __attribute__((weakref ("wv12"))); extern vtype wv12 __attribute__((weak)); #define chk(p) do { if (!p) abort (); } while (0) int main () { chk (!&Wv12); chk (!&wv12); return (0); } % gcc t.c /tmp/cciZgRxK.o:(.rodata+0x0): undefined reference to `wv12' /tmp/cciZgRxK.o:(.rodata+0x8): undefined reference to `wv12' % gcc t.c -S % cat t.s .size main, .-main * .weakref Wv12,wv12 *<< Not here. This should be after definitions of Wv12 and wv12. Wv12 .section .rodata .align 3 .LC0: .xword Wv12 .align 3 .LC1: .xword wv12 I can file a new PR and fix it by Tuesday/Wednesday or we can temporary restore Ramana's workaround in aarch64_use_blocks_for_constant_p: % git diff gcc/config/aarch64/aarch64.c diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 4b5183b..222ea33 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -7735,7 +7735,11 @@ aarch64_use_blocks_for_constant_p (machine_mode, const_rtx) { /* We can't use blocks for constants when we're using a per-function constant pool. */ - return !aarch64_can_use_per_function_literal_pools_p (); + /* Fixme:: + return !aarch64_can_use_per_function_literal_pools_p (); + This return statement breaks gcc.dg/attr-weakref-1.c test. + For now we workaround this by returning false here. */ + return false; } /* Select appropriate section for constants depending -Vladimir > > Failures: > gcc.dg/attr-weakref-1.c > > Bisected to: > > > commit 536728c16d6a0173930ecfe370302baa471c299e > Author: rguenth > Date: Thu Mar 15 08:55:04 2018 + > > 2018-03-15 Vladimir Mezentsev > > PR target/68256 > * varasm.c (hash_section): Return an unchangeble hash value > * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p): > Return !aarch64_can_use_per_function_literal_pools_p (). > > > Thanks, > bin >> ChangeLog: >> 2018-02-15 Vladimir Mezentsev >> >> PR gcc/68256 >> * varasm.c (hash_section): Return an unchangeble hash value >> * config/aarch64/aarch64.c (aarch64_use_blocks_for_constant_p): >> Return !aarch64_can_use_per_function_literal_pools_p (); >> --- >> gcc/config/aarch64/aarch64.c | 8 +++- >> gcc/varasm.c | 2 +- >> 2 files changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >> index 174310c..a0a495d 100644 >> --- a/gcc/config/aarch64/aarch64.c >> +++ b/gcc/config/aarch64/aarch64.c >> @@ -7596,11 +7596,9 @@ aarch64_can_use_per_function_literal_pools_p (void) >> static bool >> aarch64_use_blocks_for_constant_p (machine_mode, const_rtx) >> { >> - /* Fixme:: In an ideal world this would work similar >> - to the logic in aarch64_select_rtx_section but this >> - breaks bootstrap in gcc go. For now we workaround >> - this by returning false here. */ >> - return false; >> + /* We can't use blocks for constants when we're using a per-function >> + constant pool. */ >> + return !aarch64_can_use_per_function_literal_pools_p (); >> } >> >> /* Select appropriate section for constants depending >> diff --git a/gcc/varasm.c b/gcc/varasm.c >> index b045efa..5aae5b4 100644 >> --- a/gcc/varasm.c >> +++ b/gcc/varasm.c >> @@ -225,7 +225,7 @@ hash_section (section *sect) >> { >>if (sect->common.flags & SECTION_NAMED) >> return htab_hash_string (sect->named.n