RE: [PATCH][4.9] PR 64569 - Backport support for MIPS binutils 2.25
Hi Jakub, I haven't done a backport to a release branch before. Could you tell me who needs to approve this change, it only affects MIPS? Thanks, Matthew > -Original Message- > From: Matthew Fortune > Sent: 26 January 2015 16:30 > To: 'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org) > Cc: Moore, Catherine (catherine_mo...@mentor.com) > Subject: RE: [PATCH][4.9] PR 64569 - Backport support for MIPS binutils > 2.25 > > > This is a minimal backport of features added to GCC 5 to enable use of > > binutils 2.25 with GCC 4.9 for MIPS soft-float builds. Further details > > in the PR: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64569 > > > > The commits which are being backported are listed below (the last one > > is posted but not committed yet). > > > > r213870: Fix mips16.S for soft-float > > r213872: Pass -m(soft|hard|single|double)-float via ASM_SPEC > > r217446: Implement o32 FPXX (very minimal backport) > > r217939: Update configure check for HAVE_MIPS_DOT_MODULE > > r??: Make ASM_SPEC changes conditional on HAVE_MIPS_DOT_MODULE > > Updated - The last one in the list is committed as: > r219867: MIPS: Only pass floating-point options to the assembler when > necessary > > I'm not sure who to CC as RM for GCC 4.9. > > Thanks, > Matthew > > > > > gcc/ > > * config.in [!USED_FOR_TARGET] (HAVE_AS_DOT_MODULE): Undefine. > > * config/mips/mips.h (FP_ASM_SPEC): New macro. > > (ASM_SPEC): Use FP_ASM_SPEC. > > * configure.ac (HAVE_AS_DOT_MODULE): Detect support for .module > > and FPXX extensions. > > > > libgcc/ > > * config/mips/mips16.S: Do not build for soft-float. > > > > Once this is done I will do the same backport for GCC 4.8. > > > > Tested to check that soft-float builds work with binutils 2.25 and the > > floating-point options are not passed for binutils 2.24. > > > > Thanks, > > Matthew > > > > --- > > gcc/config.in | 6 ++ > > gcc/config/mips/mips.h | 19 ++- > > gcc/configure | 32 > > gcc/configure.ac| 7 +++ > > libgcc/config/mips/mips16.S | 10 +++--- > > 5 files changed, 70 insertions(+), 4 deletions(-) > > > > diff --git a/gcc/config.in b/gcc/config.in index 1e85325..013a606 > > 100644 > > --- a/gcc/config.in > > +++ b/gcc/config.in > > @@ -447,6 +447,12 @@ > > #endif > > > > > > +/* Define if the assembler understands .module. */ #ifndef > > +USED_FOR_TARGET #undef HAVE_AS_DOT_MODULE #endif > > + > > + > > /* Define if your assembler supports the -no-mul-bug-abort option. */ > > #ifndef USED_FOR_TARGET #undef HAVE_AS_NO_MUL_BUG_ABORT_OPTION diff > > --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index > > a786d4c..ff88d98 100644 > > --- a/gcc/config/mips/mips.h > > +++ b/gcc/config/mips/mips.h > > @@ -1163,6 +1163,22 @@ struct mips_cpu_info { #define > > SUBTARGET_ASM_SPEC "" > > #endif > > > > +/* FP_ASM_SPEC represents the floating-point options that must be > passed > > + to the assembler when FPXX support exists. Prior to that point > the > > + assembler could accept the options but were not required for > > + correctness. We only add the options when absolutely necessary > > + because passing -msoft-float to the assembler will cause it to > reject > > + all hard-float instructions which may require some user code to be > > + updated. */ > > + > > +#ifdef HAVE_AS_DOT_MODULE > > +#define FP_ASM_SPEC "\ > > +%{mhard-float} %{msoft-float} \ > > +%{msingle-float} %{mdouble-float}" > > +#else > > +#define FP_ASM_SPEC > > +#endif > > + > > #undef ASM_SPEC > > #define ASM_SPEC "\ > > %{G*} %(endian_spec) %{mips1} %{mips2} %{mips3} %{mips4} \ @@ -1188,7 > > +1204,8 @@ struct mips_cpu_info { %{mfp32} %{mfp64} %{mnan=*} \ > > %{mshared} %{mno-shared} \ %{msym32} %{mno-sym32} \ -%{mtune=*} \ > > +%{mtune=*}" \ > > +FP_ASM_SPEC "\ > > %(subtarget_asm_spec)" > > > > /* Extra switches sometimes passed to the linker. */ diff --git > > a/gcc/configure b/gcc/configure index 291e463..d5b6879 100755 > > --- a/gcc/configure > > +++ b/gcc/configure > > @@ -26140,6 +26140,38 @@ $as_echo "#define HAVE_AS_GNU_ATTRIBUTE 1" > > >>confdefs.h > > > > fi > > > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for > > .module support" >&5 > > +$as_echo_n "checking assembler for .module support... " >&6; } if > > +test "${gcc_cv_as_mips_dot_module+set}" = set; then : > > + $as_echo_n "(cached) " >&6 > > +else > > + gcc_cv_as_mips_dot_module=no > > + if test x$gcc_cv_as != x; then > > +$as_echo '.module mips2 > > + .module fp=xx' > conftest.s > > +if { ac_try='$gcc_cv_as $gcc_cv_as_flags -32 -o conftest.o > > +conftest.s > > >&5' > > + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } > > +>&5 > > + (eval $ac_try) 2>&5 > > + ac_status=$? > > + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 > > + test $ac_status = 0; }; } > > +then > > + gcc_cv_as_mips_d
Re: [PATCH] PR rtl-optimization/32219: optimizer causes wrong code in pic/hidden/weak symbol checking
H.J., The new patch bootstraps okay on x86_64-apple-darwin14 but I discovered that you need a small adjustment in the deja-gnu statements... --- /Users/howarth/gcc-5-20150206/gcc/testsuite/gcc.dg/visibility-22.c 2015-02-06 21:45:04.0 -0500 +++ /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/gcc.dg/visibility-22.c 2015-02-07 03:24:42.0 -0500 @@ -8,9 +8,9 @@ /* For kernel modules and static RTPs, the loader treats undefined weak symbols in the same way as undefined strong symbols. The test therefore fails to load, so skip it. */ +/* { dg-options "-fPIC" { target fpic } } */ /* { dg-additional-options "-Wl,-undefined,dynamic_lookup" { target *-*-darwin* } } */ /* { dg-additional-options "-Wl,-flat_namespace" { target *-*-darwin[89]* } } */ -/* { dg-options "-fPIC" { target fpic } } */ extern void foo () __attribute__((weak,visibility("hidden"))); int If you don't define a dg-options line first, the dg-additional-options lines have no effect. Jack On Fri, Feb 6, 2015 at 8:55 PM, H.J. Lu wrote: > On Fri, Feb 6, 2015 at 5:51 PM, Jack Howarth wrote: >> H.J., >> This patch also seems to be causing a huge number of regressions >> in the g++ test suite due to linkage warnings on darwin of the form... >> >> ld: warning: direct access in Model::~Model() to global weak symbol >> vtable for Model means the weak symbol cannot be overridden at >> runtime. This was likely caused by different translation units being >> compiled with different visibility settings. > > Can you try my new patch? > >> Can this change wait until stage1? >> Jack >> >> On Fri, Feb 6, 2015 at 4:41 PM, H.J. Lu wrote: >>> On Fri, Feb 6, 2015 at 1:31 PM, Jack Howarth >>> wrote: H.J., On x86_64-apple-darwin14, your patch applied to r220481 results in... FAIL: gcc.dg/visibility-22.c (test for excess errors) FAIL: gcc.dg/visibility-23.c scan-hidden private_extern[ \t_]*_?foo with... Executing on host: /sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/xgcc -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/ /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/gcc.dg/visibility-22.c -fno-diagnostics-show-caret -fdiagnostics-color=never -fPIC -lm -m32 -o ./visibility-22.exe(timeout = 300) spawn -ignore SIGHUP /sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/xgcc -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/ /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/gcc.dg/visibility-22.c -fno-diagnostics-show-caret -fdiagnostics-color=never -fPIC -lm -m32 -o ./visibility-22.exe^M Undefined symbols for architecture i386:^M "_foo", referenced from:^M _main in ccMD1qjz.o^M _main in ccMD1qjz.o^M ld: symbol(s) not found for architecture i386^M collect2: error: ld returned 1 exit status^M compiler exited with status 1 output is: Undefined symbols for architecture i386:^M "_foo", referenced from:^M _main in ccMD1qjz.o^M _main in ccMD1qjz.o^M ld: symbol(s) not found for architecture i386^M collect2: error: ld returned 1 exit status^M FAIL: gcc.dg/visibility-22.c (test for excess errors) Executing on host: /sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/xgcc -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/ /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/gcc.dg/visibility-23.c -fno-diagnostics-show-caret -fdiagnostics-color=never -fPIC -S -m32 -o visibility-23.s(timeout = 300) spawn -ignore SIGHUP /sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/xgcc -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/ /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/gcc.dg/visibility-23.c -fno-diagnostics-show-caret -fdiagnostics-color=never -fPIC -S -m32 -o visibility-23.s^M PASS: gcc.dg/visibility-23.c (test for excess errors) FAIL: gcc.dg/visibility-23.c scan-hidden private_extern[ \t_]*_?foo >>> >>> Does Darwin support undefined hidden weak symbol? >>> Can you compile and gcc/testsuite/gcc.dg/visibility-22.c >>> with clang on Darwin? >>> >>> -- >>> H.J. > > > > -- > H.J.
Re: RFA: RL78: Fix gcc testsuite failures
On Wed, 2015-02-04 11:14:16 +, Nick Clifton wrote: > Please can I apply the patch below to fix some RL78 gcc testsuite > failures ? [...] > The second fix is to the RL78 specific dead-code elimination pass > which was failing to note the REGs inside a MEM are used when that MEM > is the destination of a SET insn. > > Tested with no regressions and lots of fixes using an rl78-elf > toolchain. > > OK to apply ? Seems you accidentally committed quite some more code you're currently working on in that very commit, which is now breaking (see eg. http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=417627): [...] g++ -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I. -I. -I/home/jbglaw/repos/gcc/gcc -I/home/jbglaw/repos/gcc/gcc/. -I/home/jbglaw/repos/gcc/gcc/../include -I/home/jbglaw/repos/gcc/gcc/../libcpp/include -I/home/jbglaw/repos/gcc/gcc/../libdecnumber -I/home/jbglaw/repos/gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I/home/jbglaw/repos/gcc/gcc/../libbacktrace -o rl78.o -MT rl78.o -MMD -MP -MF ./.deps/rl78.TPo /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c: In function ‘tree_node* rl78_handle_func_attribute(tree_node**, tree_node*, tree_node*, int, bool*)’: /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:728: warning: unknown conversion type character ‘E’ in format /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:728: warning: too many arguments for format /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c: In function ‘tree_node* rl78_handle_saddr_attribute(tree_node**, tree_node*, tree_node*, int, bool*)’: /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:775: warning: unknown conversion type character ‘E’ in format /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:775: warning: too many arguments for format /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c: In function ‘void rl78_alloc_physical_registers()’: /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:3465: error: ‘VALLOC_DIVHI’ was not declared in this scope /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:3470: error: ‘VALLOC_DIVSI’ was not declared in this scope /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:3512: error: ‘VALLOC_DIVSI’ was not declared in this scope /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:3519: error: ‘VALLOC_DIVHI’ was not declared in this scope /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c: In function ‘void set_origin(rtx_def*, rtx_insn*, int*, int*)’: /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:3838: error: ‘VALLOC_DIVHI’ was not declared in this scope /home/jbglaw/repos/gcc/gcc/config/rl78/rl78.c:3857: error: ‘VALLOC_DIVSI’ was not declared in this scope make[1]: *** [rl78.o] Error 1 With a more recent compiler, that condenses to: [...] g++ -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace -o rl78.o -MT rl78.o -MMD -MP -MF ./.deps/rl78.TPo ../../gcc/gcc/config/rl78/rl78.c ../../gcc/gcc/config/rl78/rl78.c: In function ‘void rl78_alloc_physical_registers()’: ../../gcc/gcc/config/rl78/rl78.c:3465:33: error: ‘VALLOC_DIVHI’ was not declared in this scope else if (valloc_method == VALLOC_DIVHI) ^ ../../gcc/gcc/config/rl78/rl78.c:3470:33: error: ‘VALLOC_DIVSI’ was not declared in this scope else if (valloc_method == VALLOC_DIVSI) ^ ../../gcc/gcc/config/rl78/rl78.c:3512:7: error: ‘VALLOC_DIVSI’ was not declared in this scope case VALLOC_DIVSI: ^ ../../gcc/gcc/config/rl78/rl78.c:3519:7: error: ‘VALLOC_DIVHI’ was not declared in this scope case VALLOC_DIVHI: ^ ../../gcc/gcc/config/rl78/rl78.c: In function ‘void set_origin(rtx, rtx_insn*, int*, int*)’: ../../gcc/gcc/config/rl78/rl78.c:3838:38: error: ‘VALLOC_DIVHI’ was not declared in this scope else if (get_attr_valloc (insn) == VALLOC_DIVHI) ^ ../../gcc/gcc/config/rl78/rl78.c:3857:38: error: ‘VALLOC_DIVSI’ was not declared in this scope else if (get_attr_valloc (insn) == VALLOC_DIVSI) ^ make[1]: *** [rl78.o] Error 1 MfG, JBG -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of: Alles wird gut! ...und heute wirds schon ein bißchen besser. th
Re: [PATCH] Update optimization_default_node in ada (PR middle-end/64340)
On February 6, 2015 9:26:29 PM CET, Jakub Jelinek wrote: >Hi! > >As mentioned in the PR, the problem here is that the Ada FE needs to >modify >global_options after toplevel.c (process_options), but as for LTO we >now use >optimization_{default,current}_node for options of functions without >specifial optimization node, it means the changed options aren't >reflected >in there. > >Fixed thusly, bootstrapped/regtested on x86_64-linux, ok for trunk? Can't ada use one of the option processing langhooks or represent this change similar to optimization pragmas in other frontends? Richard. >2015-02-06 Jakub Jelinek > > PR middle-end/64340 > * gcc-interface/trans.c (gigi): Recreate optimization_default_node > and optimization_current_node after tweaking global_options. > >--- gcc/ada/gcc-interface/trans.c.jj 2015-01-12 18:56:10.0 >+0100 >+++ gcc/ada/gcc-interface/trans.c 2015-02-06 16:39:04.353363393 +0100 >@@ -678,6 +678,11 @@ gigi (Node_Id gnat_root, > if (No_Strict_Aliasing_CP) > flag_strict_aliasing = 0; > >+ /* Save the current optimization options again after the above >possible >+ global_options changes. */ >+ optimization_default_node = build_optimization_node >(&global_options); >+ optimization_current_node = optimization_default_node; >+ > /* Now translate the compilation unit proper. */ > Compilation_Unit_to_gnu (gnat_root); > > > Jakub
Re: [PATCH] PR 64256
On February 7, 2015 7:50:38 AM CET, Jeff Law wrote: >On 02/06/15 19:32, David Edelsohn wrote: >> After a lot of investigation, I believe that I have learned why stabs >> debugging on AIX disabled use of continuations. GDB, IBM DBX and IBM >> XLDB are able to work with stab string continuations produced by GCC. >> >> I am enabling it using definitions that match the behavior of IBM XL >> compilers to avoid any unexpected behavior and hopefully be able to >> reproduce any future errors with native tools to expedite fixes. >> XCOFF32 uses an unsigned short for the stabstring length for a >maximum >> length of 64K, but XLC limits the size to 16K. XLC also uses '?' as >a >> continuation character instead of '\\'. GDB accepts '?' as an >> alternate continuation character. >> >> Bootstrapped on powerpc-ibm-aix7.1.0.0. >> >> Committed. >> >> Thanks, David >> >> PR debug/2714 >> PR bootstrap/64256 >> * xcoffout.h (DBX_CONTIN_LENGTH): Define as 16384. >> (DBX_CONTIN_CHAR): Define. >Well, I thought I was going to have the oldest BZ fixed for this >release, but you beat me by a mile. > >Glad to see this resolved. Indeed. Please also consider back porting this to release branches. Thanks, Richard. >jeff
Re: [PATCH] Update optimization_default_node in ada (PR middle-end/64340)
On Sat, Feb 07, 2015 at 11:28:54AM +0100, Richard Biener wrote: > On February 6, 2015 9:26:29 PM CET, Jakub Jelinek wrote: > >Hi! > > > >As mentioned in the PR, the problem here is that the Ada FE needs to > >modify > >global_options after toplevel.c (process_options), but as for LTO we > >now use > >optimization_{default,current}_node for options of functions without > >specifial optimization node, it means the changed options aren't > >reflected > >in there. > > > >Fixed thusly, bootstrapped/regtested on x86_64-linux, ok for trunk? > > Can't ada use one of the option processing langhooks or represent this change > similar to optimization pragmas in other frontends? I think for using option processing langhooks it would require massive Ada FE surgery, the thing is that right now all the option processing is performed, then at some later point the TU is parsed, then depending on what is seen in there the options are tweaked and finally everything is handed over to the middle-end. So, to do this in the option processing langhooks, the parsing (which presumably depends on the user options etc.) would need to be performed during the option processing. As for the other option, sure, instead of recreating optimization_{default,current}_node as done in the patch, it could create a different OPTIMIZATION_NODE instead, and attach it to all functions. What would be optimization_default_node good for it isn't used for anything though? And, the thing I'm worried about in that case is how to ensure the ada_optimization_default_node or how would it be called optimization node is used even on all compiler generated functions, not just user functions. So, from this POV my patch sounds like the simplest solution, both the above options would be IMHO significantly more work. If some Ada maintainer is willing to do that, sure, no problem with that, but I know next to nothing about Ada and thus I'm afraid what I've posted is all I can offer for this PR. Jakub
Re: [PATCH] Update optimization_default_node in ada (PR middle-end/64340)
> I think for using option processing langhooks it would require massive Ada > FE surgery, the thing is that right now all the option processing is > performed, then at some later point the TU is parsed, then depending on what > is seen in there the options are tweaked and finally everything is handed > over to the middle-end. So, to do this in the option processing langhooks, > the parsing (which presumably depends on the user options etc.) would need > to be performed during the option processing. > > As for the other option, sure, instead of recreating > optimization_{default,current}_node as done in the patch, it could create > a different OPTIMIZATION_NODE instead, and attach it to all functions. > What would be optimization_default_node good for it isn't used for anything > though? And, the thing I'm worried about in that case is how to ensure the > ada_optimization_default_node or how would it be called optimization node is > used even on all compiler generated functions, not just user functions. > > So, from this POV my patch sounds like the simplest solution, both the above > options would be IMHO significantly more work. If some Ada maintainer is > willing to do that, sure, no problem with that, but I know next to nothing > about Ada and thus I'm afraid what I've posted is all I can offer for this > PR. I don't think that the langhook approach is realistically doable, unless of course you want to add a new dedicated langhook. The Ada compiler needs to adjust the behavior of the middle-end and the optimizers after parsing the compilation unit and this cannot really be changed. The second approach is more appealing but I'll defer to your judgement here because you know this stuff much better than me. To sum up, if you think that the patch is a plausible kludge, then go ahead. -- Eric Botcazou
Re: [PATCH] PR rtl-optimization/32219: optimizer causes wrong code in pic/hidden/weak symbol checking
On Sat, Feb 07, 2015 at 03:28:38AM -0500, Jack Howarth wrote: > H.J., > The new patch bootstraps okay on x86_64-apple-darwin14 but I Does it cause any regressions on x86_64-apple-darwin14? > discovered that you need a small adjustment in the deja-gnu > statements... > > --- /Users/howarth/gcc-5-20150206/gcc/testsuite/gcc.dg/visibility-22.c > 2015-02-06 21:45:04.0 -0500 > +++ > /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/gcc.dg/visibility-22.c > 2015-02-07 03:24:42.0 -0500 > @@ -8,9 +8,9 @@ > /* For kernel modules and static RTPs, the loader treats undefined weak > symbols in the same way as undefined strong symbols. The test > therefore fails to load, so skip it. */ > +/* { dg-options "-fPIC" { target fpic } } */ > /* { dg-additional-options "-Wl,-undefined,dynamic_lookup" { target > *-*-darwin* } } */ > /* { dg-additional-options "-Wl,-flat_namespace" { target > *-*-darwin[89]* } } */ > -/* { dg-options "-fPIC" { target fpic } } */ > > extern void foo () __attribute__((weak,visibility("hidden"))); > int > > If you don't define a dg-options line first, the dg-additional-options > lines have no effect. > Jack Here is the updated patch. H.J. --- >From 8e61705db8177d41fac45dbeffa4faf6163d4c94 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Thu, 5 Feb 2015 14:28:58 -0800 Subject: [PATCH] Handle symbol visibility/locality for PIE/PIC If a hidden weak symbol isn't defined in the TU, we can't assume it will be defined in another TU at link time. It makes a difference in code generation when compiling for PIC. If we assume that a hidden weak undefined symbol is local, the address checking may be optimized out and leads to the wrong code. This means that a symbol with user specified visibility is local only if it is locally resolved or defined, not weak or not compiling for PIC. When symbol visibility is specified in the source, we should always output symbol visibility even if symbol isn't local to the TU. If a global data symbol is defined in the TU, it is always local to the executable, regardless if it is a common symbol or not. If we aren't compiling for shared library, locally defined global data symbol binds locally. gcc/ PR rtl-optimization/32219 * cgraphunit.c (varpool_node::finalize_decl): Set definition first before calling notice_global_symbol so that it is available to notice_global_symbol. * varasm.c (default_binds_local_p_1): Resolve defined data symbol locally if not building shared library. Resolve symbol with user specified visibility locally only if it is locally resolved or defined, not weak or not compiling for PIC. (default_elf_asm_output_external): Always output visibility specified in the source. * config/darwin-protos.h (darwin_output_external): New. * config/darwin.c (darwin_output_external): Likewise. * config/darwin.h (ASM_OUTPUT_EXTERNAL): Likewise. gcc/testsuite/ PR rtl-optimization/32219 * gcc.dg/visibility-22.c: New test. * gcc.dg/visibility-23.c: Likewise. * gcc.target/i386/pr32219-1.c: Likewise. * gcc.target/i386/pr32219-2.c: Likewise. * gcc.target/i386/pr32219-3.c: Likewise. * gcc.target/i386/pr32219-4.c: Likewise. * gcc.target/i386/pr32219-5.c: Likewise. * gcc.target/i386/pr32219-6.c: Likewise. * gcc.target/i386/pr32219-7.c: Likewise. * gcc.target/i386/pr32219-8.c: Likewise. * gcc.target/i386/pr64317.c: Expect GOTOFF relocation instead of GOT relocation. --- gcc/cgraphunit.c | 4 +++- gcc/config/darwin-protos.h| 1 + gcc/config/darwin.c | 15 + gcc/config/darwin.h | 9 gcc/testsuite/gcc.dg/visibility-22.c | 22 +++ gcc/testsuite/gcc.dg/visibility-23.c | 14 + gcc/testsuite/gcc.target/i386/pr32219-1.c | 16 ++ gcc/testsuite/gcc.target/i386/pr32219-2.c | 16 ++ gcc/testsuite/gcc.target/i386/pr32219-3.c | 17 +++ gcc/testsuite/gcc.target/i386/pr32219-4.c | 17 +++ gcc/testsuite/gcc.target/i386/pr32219-5.c | 16 ++ gcc/testsuite/gcc.target/i386/pr32219-6.c | 16 ++ gcc/testsuite/gcc.target/i386/pr32219-7.c | 17 +++ gcc/testsuite/gcc.target/i386/pr32219-8.c | 17 +++ gcc/testsuite/gcc.target/i386/pr64317.c | 2 +- gcc/varasm.c | 35 ++- 16 files changed, 217 insertions(+), 17 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/visibility-22.c create mode 100644 gcc/testsuite/gcc.dg/visibility-23.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-1.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-2.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-3.c creat
Re: [Patch, fortran] PR63744 accept duplicate use-rename
Dear Mikael, It looks good to me for trunk and the branches. Thanks for the patch Paul On 6 February 2015 at 21:31, Mikael Morin wrote: > Hello, > > we currently reject programs of the form >> >> module m >> integer :: s >> end module m >> subroutine s >> use m, only: x => s, x => s >> end subroutine s > > with an error stating that S is the name of the current program unit. > Interestingly, the duplicate rename is necessary to trigger it. > > There doesn't seem to be a consensus as to whether it should be > accepted, but I think it should be. > Quoting Dominique's comment in the PR: >> if >> >> use m, only: A => X >> use m, only: B => X >> >> is valid, I don't see why >> >> use m, only: A => X >> use m, only: A => X >> >> should not. > The problem is we check the original (symbol) name instead of the local > (symtree) name. > The fix is close to obvious, and should be safe for the branches (this > is a regression) > Regression tested on x86_64-linux. OK for trunk/4.9/4.8 ? > > Mikael > > > -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
[Patch, fortran] PR64932 [4.9/5 Regression] ICE in gfc_conv_descriptor_data_get for generated finalizer
Dear All, This is a slight development of the patch posted on the PR itself. class.c(finalize_component) is not able to deal correctly with non-allocatable, derived type array components that have allocatable components. Rather than generating loops in finalize_component, the condition is detected in trans-stmt.c(gfc_trans_deallocate) and gfc_deallocate_alloc_comp is called after obtaining the derived type for the array and checking that it is not finalizable. Happily, this fix does not generate the error: Error: Two or more part references with nonzero rank must not be specified at (1) which occurs if the code is written explicitly. Bootstraps and regtests on FC21/x86_64 OK for trunk and 4.9? Paul 2015-02-07 Paul Thomas PR fortran/64932 * trans-stmt.c (gfc_trans_deallocate): If a component array expression is not a descriptor type and it is a derived type that has allocatable components and is not finalizable, then deallocate the allocatable components. 2015-02-07 Paul Thomas PR fortran/6432 * gfortran.dg/finalize_28.f90: New test Index: gcc/fortran/trans-stmt.c === *** gcc/fortran/trans-stmt.c(revision 220481) --- gcc/fortran/trans-stmt.c(working copy) *** gfc_trans_deallocate (gfc_code *code) *** 5575,5585 if (expr->rank || gfc_is_coarray (expr)) { if (expr->ts.type == BT_DERIVED && expr->ts.u.derived->attr.alloc_comp && !gfc_is_finalizable (expr->ts.u.derived, NULL)) { - gfc_ref *ref; gfc_ref *last = NULL; for (ref = expr->ref; ref; ref = ref->next) if (ref->type == REF_COMPONENT) last = ref; --- 5575,5587 if (expr->rank || gfc_is_coarray (expr)) { + gfc_ref *ref; + if (expr->ts.type == BT_DERIVED && expr->ts.u.derived->attr.alloc_comp && !gfc_is_finalizable (expr->ts.u.derived, NULL)) { gfc_ref *last = NULL; + for (ref = expr->ref; ref; ref = ref->next) if (ref->type == REF_COMPONENT) last = ref; *** gfc_trans_deallocate (gfc_code *code) *** 5590,5602 && !(!last && expr->symtree->n.sym->attr.pointer)) { tmp = gfc_deallocate_alloc_comp (expr->ts.u.derived, se.expr, ! expr->rank); gfc_add_expr_to_block (&se.pre, tmp); } } ! tmp = gfc_array_deallocate (se.expr, pstat, errmsg, errlen, ! label_finish, expr); ! gfc_add_expr_to_block (&se.pre, tmp); if (al->expr->ts.type == BT_CLASS) gfc_reset_vptr (&se.pre, al->expr); } --- 5592,5636 && !(!last && expr->symtree->n.sym->attr.pointer)) { tmp = gfc_deallocate_alloc_comp (expr->ts.u.derived, se.expr, ! expr->rank); gfc_add_expr_to_block (&se.pre, tmp); } } ! ! if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (se.expr))) ! { ! tmp = gfc_array_deallocate (se.expr, pstat, errmsg, errlen, ! label_finish, expr); ! gfc_add_expr_to_block (&se.pre, tmp); ! } ! else if (TREE_CODE (se.expr) == COMPONENT_REF ! && TREE_CODE (TREE_TYPE (se.expr)) == ARRAY_TYPE ! && TREE_CODE (TREE_TYPE (TREE_TYPE (se.expr))) ! == RECORD_TYPE) ! { ! /* class.c(finalize_component) generates these, when a !finalizable entity has a non-allocatable derived type array !component, which has allocatable components. Obtain the !derived type of the array and deallocate the allocatable !components. */ ! for (ref = expr->ref; ref; ref = ref->next) ! { ! if (ref->u.c.component->attr.dimension ! && ref->u.c.component->ts.type == BT_DERIVED) ! break; ! } ! ! if (ref && ref->u.c.component->ts.u.derived->attr.alloc_comp ! && !gfc_is_finalizable (ref->u.c.component->ts.u.derived, ! NULL)) ! { ! tmp = gfc_deallocate_alloc_comp ! (ref->u.c.component->ts.u.derived, !se.expr, expr->rank); ! gfc_add_expr_to_block (&se.pre, tmp); ! } ! } ! if (al->expr->ts.type == BT_CLASS) gfc_reset_vptr (&se.pre, al->expr); } Index: gcc/testsuite/gfortran.dg/finalize_28.f90
Re: [Patch, fortran] PR63744 accept duplicate use-rename
Dear Mikael, Even if >> use m, only: A => X >> use m, only: A => X is valid, it does not make sense to use it and it is probably a typo. Should not gfortran emit a warning, at least with -Wall? Cheers, Dominique
Re: [PATCH] Update optimization_default_node in ada (PR middle-end/64340)
On February 7, 2015 11:55:23 AM CET, Eric Botcazou wrote: >> I think for using option processing langhooks it would require >massive Ada >> FE surgery, the thing is that right now all the option processing is >> performed, then at some later point the TU is parsed, then depending >on what >> is seen in there the options are tweaked and finally everything is >handed >> over to the middle-end. So, to do this in the option processing >langhooks, >> the parsing (which presumably depends on the user options etc.) would >need >> to be performed during the option processing. >> >> As for the other option, sure, instead of recreating >> optimization_{default,current}_node as done in the patch, it could >create >> a different OPTIMIZATION_NODE instead, and attach it to all >functions. >> What would be optimization_default_node good for it isn't used for >anything >> though? And, the thing I'm worried about in that case is how to >ensure the >> ada_optimization_default_node or how would it be called optimization >node is >> used even on all compiler generated functions, not just user >functions. >> >> So, from this POV my patch sounds like the simplest solution, both >the above >> options would be IMHO significantly more work. If some Ada >maintainer is >> willing to do that, sure, no problem with that, but I know next to >nothing >> about Ada and thus I'm afraid what I've posted is all I can offer for >this >> PR. > >I don't think that the langhook approach is realistically doable, >unless of >course you want to add a new dedicated langhook. The Ada compiler >needs to >adjust the behavior of the middle-end and the optimizers after parsing >the >compilation unit and this cannot really be changed. > >The second approach is more appealing but I'll defer to your judgement >here >because you know this stuff much better than me. > >To sum up, if you think that the patch is a plausible kludge, then go >ahead. OK with me as well then. Richard.
Re: [patch, libgfortran] [4.8/4.9/5 Regression] error reading (and writing) large text files in gfortran
Dear Jerry, This is OK for trunk. Maybe it is best to leave it for a week or two before committing to the branches? Thanks for the patch. Paul On 7 February 2015 at 03:08, Jerry DeLisle wrote: > With the attached patch I create a special version of fbuf_flush that is > only called with list directed I/O. There can be no tabbing back and forth > so it is safe to flush the buffer whenever we want. The bug occurs when the > buffer keeps growing to no end until no more allocations can be made and the > OS gives an error. > > fbuf_flush can be called as much as one wants to, but to keep overhead low, > I introduce a new version with an arbitrary limit. If below the limit, just > return doing no flushing. When the limit is exceeded, fbuf is flushed. The > code to do this is duplicated from the original fbuf_flush so it is very > safe and well tested. > > I played with the allowable maximum position limit for flushing and ran some > timing tests. My machine here uses a solid state drive which may bias the > results somewhat. Others are welcome to test and see what values they get > as well. I settled on 524588 based on these results, favoring writing over > reading a little. > > The patch has zero impact on any other type of I/O including normal > formatted I/O. I also tested to confirm that formatted I/O does not have > the problem. It is flushed regularly in the main transfer loop. As far as > I can tell only list directed has the issue. > > I get the following timings using the attached test program. > > WRITING: > Array Size--> 100 1 > Buf Limit > > 16384 2.107 210.9 > 32768 2.026 292.1 > 65636 2.232 235.8 > 5242881.958 193.5 > 10485762.023 203.5 > > > READING: > Buf Limit > > 16384 1.843 184.4 > 32768 1.841 186.8 > 65636 1.816 197.6 > 5242881.879 186.5 > 10485761.834 185.2 > > Regression tested on x86-64 Linux. > > OK for trunk followed by backports? > > I can not include a specific testsuite test case, it would take way too long > to run. > > Regards, > > Jerry > > 2015-02-07 Jerry DeLisle > > PR libgfortran/60956 > * io/fbuf.c (fbuf_flush_list): New function that only flushes > if current fbuf position exceeds a limit. > * io/fbuf.h: Declare the new function. > * io/io.h (enum unit_mode): Add two new modes. > * io/list_read.c (list_formatted_read_scalar): Call new function. > * io/write.c: Include fbuf.h. (list_formatted_write_scalar): > Call new function. -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [PATCH] PR rtl-optimization/32219: optimizer causes wrong code in pic/hidden/weak symbol checking
H.J,, Unfortunately, the answer is yes. This patch still introduces regressions in the g++ test suite.l These are all some form of... Executing on host: /sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/testsuite/g++/../../xg++ -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/testsuite/g++/../../ /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/g++.dg/abi/empty7.C -fno-diagnostics-show-caret -fdiagnostics-color=never -nostdinc++ -I/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/include/x86_64-apple-darwin14.3.0 -I/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/include -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/libsupc++ -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/include/backward -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/testsuite/util -fmessage-length=0 -std=gnu++98 -fabi-version=0 -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -multiply_defined suppress -lm -m32 -o ./empty7.exe(timeout = 300) spawn -ignore SIGHUP /sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/testsuite/g++/../../xg++ -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/testsuite/g++/../../ /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/g++.dg/abi/empty7.C -fno-diagnostics-show-caret -fdiagnostics-color=never -nostdinc++ -I/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/include/x86_64-apple-darwin14.3.0 -I/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/include -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/libsupc++ -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/include/backward -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/testsuite/util -fmessage-length=0 -std=gnu++98 -fabi-version=0 -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -multiply_defined suppress -lm -m32 -o ./empty7.exe^M ld: warning: direct access in S2::S2()to global weak symbol vtable for S2 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M ld: warning: direct access in S5::S5()to global weak symbol vtable for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M ld: warning: direct access in S5::S5()to global weak symbol vtable for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M ld: warning: direct access in S5::S5()to global weak symbol vtable for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M ld: warning: direct access in S5::S5()to global weak symbol VTT for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M ld: warning: direct access in S5::S5()to global weak symbol VTT for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M ld: warning: direct access in S8::S8()to global weak symbol vtable for S8 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M output is: ld: warning: direct access in S2::S2()to global weak symbol vtable for S2 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M ld: warning: direct access in S5::S5()to global weak symbol vtable for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.^M ld: warning: direct access in S5::S5()to global weak symbol vtable for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibili
Re: [PATCH] PR rtl-optimization/32219: optimizer causes wrong code in pic/hidden/weak symbol checking
H.J., In case it clarifies things at all, the falling testcase... % /sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/testsuite/g++/../../xg++ -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/gcc/testsuite/g++/../../ /sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/gcc/testsuite/g++.dg/abi/empty7.C -fno-diagnostics-show-caret -fdiagnostics-color=never -nostdinc++ -I/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/include/x86_64-apple-darwin14.3.0 -I/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/include -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/libsupc++ -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/include/backward -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5-20150206/libstdc++-v3/testsuite/util -fmessage-length=0 -std=gnu++98 -fabi-version=0 -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin14.3.0/i386/libstdc++-v3/src/.libs -multiply_defined suppress -lm -m32 -o ./empty7.exe produces the linker warnings... ld: warning: direct access in S2::S2()to global weak symbol vtable for S2 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings. ld: warning: direct access in S5::S5()to global weak symbol vtable for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings. ld: warning: direct access in S5::S5()to global weak symbol vtable for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings. ld: warning: direct access in S5::S5()to global weak symbol vtable for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings. ld: warning: direct access in S5::S5()to global weak symbol VTT for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings. ld: warning: direct access in S5::S5()to global weak symbol VTT for S5 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings. ld: warning: direct access in S8::S8()to global weak symbol vtable for S8 means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings. when linking the code from empty7.o generated the following assembly in empty7.s .section __TEXT,__textcoal_nt,coalesced,pure_instructions .align 1 .globl __ZN2S21fEv .weak_definition __ZN2S21fEv __ZN2S21fEv: LFB0: pushl %ebp LCFI0: movl%esp, %ebp LCFI1: call___x86.get_pc_thunk.ax L1$pb: nop popl%ebp LCFI2: ret LFE0: .align 1 .globl __ZN2S7C2Ev .weak_definition __ZN2S7C2Ev __ZN2S7C2Ev: LFB3: pushl %ebp LCFI3: movl%esp, %ebp LCFI4: call___x86.get_pc_thunk.ax L2$pb: nop popl%ebp LCFI5: ret LFE3: .align 1 .globl __ZN2S1C2Ev .weak_definition __ZN2S1C2Ev __ZN2S1C2Ev: LFB6: pushl %ebp LCFI6: movl%esp, %ebp LCFI7: call___x86.get_pc_thunk.ax L3$pb: nop popl%ebp LCFI8: ret LFE6: .align 1 .globl __ZN2S2C2Ev .weak_definition __ZN2S2C2Ev __ZN2S2C2Ev: LFB11: pushl %ebp LCFI9: movl%esp, %ebp LCFI10: call___x86.get_pc_thunk.ax L4$pb: movl8(%ebp), %edx leal__ZTV2S2-L4$pb(%eax), %eax leal8(%eax), %eax movl%eax, (%edx) nop popl%ebp LCFI11: ret LFE11: .align 1 .globl __ZN2S4C2Ev .weak_definition __ZN2S4C2Ev __ZN2S4C2Ev: LFB14: pushl %ebp LCFI12: movl%esp, %ebp LCFI13: call___x86.get_pc_thunk.ax L5$pb: movl12(%ebp), %eax movl(%eax), %edx movl8(%ebp), %eax movl%edx, (%eax) movl8(%ebp), %eax movl(%eax), %eax subl$12, %eax movl(%eax), %eax movl%eax, %edx movl8(%ebp), %eax addl%eax, %edx movl12(%ebp), %eax movl4(%eax), %eax movl%eax, (%edx) nop popl%ebp LCFI14: ret LFE14:
Re: [PATCH] PR rtl-optimization/32219: optimizer causes wrong code in pic/hidden/weak symbol checking
On Sat, Feb 07, 2015 at 10:11:01AM -0500, Jack Howarth wrote: > H.J,, > Unfortunately, the answer is yes. This patch still introduces > regressions in the g++ test suite.l These are all some form of... > It looks like Darwin depends on the old behavior of default_binds_local_p_1. Please try this patch. H.J. >From 2df2188c97760ed10a85bff3dfef8198e41862f2 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Thu, 5 Feb 2015 14:28:58 -0800 Subject: [PATCH] Handle symbol visibility/locality for PIE/PIC If a hidden weak symbol isn't defined in the TU, we can't assume it will be defined in another TU at link time. It makes a difference in code generation when compiling for PIC. If we assume that a hidden weak undefined symbol is local, the address checking may be optimized out and leads to the wrong code. This means that a symbol with user specified visibility is local only if it is locally resolved or defined, not weak or not compiling for PIC. When symbol visibility is specified in the source, we should always output symbol visibility even if symbol isn't local to the TU. If a global data symbol is defined in the TU, it is always local to the executable, regardless if it is a common symbol or not. If we aren't compiling for shared library, locally defined global data symbol binds locally. Since some targets call default_binds_local_p_1 directly and depend on the old behavior of default_binds_local_p_1. This patch copies default_binds_local_p_1 to default_binds_local_p_2 and changes the behavior of default_binds_local_p by calling default_binds_local_p_2 instead of default_binds_local_p_1. gcc/ PR rtl-optimization/32219 * cgraphunit.c (varpool_node::finalize_decl): Set definition first before calling notice_global_symbol so that it is available to notice_global_symbol. * varasm.c (default_binds_local_p_2): Copied from default_binds_local_p_1. Resolve defined data symbol locally if not building shared library. Resolve symbol with user specified visibility locally only if it is locally resolved or defined, not weak or not compiling for PIC. (default_binds_local_p): Replace default_binds_local_p_1 with default_binds_local_p_2. (default_elf_asm_output_external): Always output visibility specified in the source. gcc/testsuite/ PR rtl-optimization/32219 * gcc.dg/visibility-22.c: New test. * gcc.dg/visibility-23.c: Likewise. * gcc.target/i386/pr32219-1.c: Likewise. * gcc.target/i386/pr32219-2.c: Likewise. * gcc.target/i386/pr32219-3.c: Likewise. * gcc.target/i386/pr32219-4.c: Likewise. * gcc.target/i386/pr32219-5.c: Likewise. * gcc.target/i386/pr32219-6.c: Likewise. * gcc.target/i386/pr32219-7.c: Likewise. * gcc.target/i386/pr32219-8.c: Likewise. * gcc.target/i386/pr64317.c: Expect GOTOFF relocation instead of GOT relocation. --- gcc/cgraphunit.c | 4 +- gcc/testsuite/gcc.dg/visibility-22.c | 17 ++ gcc/testsuite/gcc.dg/visibility-23.c | 15 + gcc/testsuite/gcc.target/i386/pr32219-1.c | 16 ++ gcc/testsuite/gcc.target/i386/pr32219-2.c | 16 ++ gcc/testsuite/gcc.target/i386/pr32219-3.c | 17 ++ gcc/testsuite/gcc.target/i386/pr32219-4.c | 17 ++ gcc/testsuite/gcc.target/i386/pr32219-5.c | 16 ++ gcc/testsuite/gcc.target/i386/pr32219-6.c | 16 ++ gcc/testsuite/gcc.target/i386/pr32219-7.c | 17 ++ gcc/testsuite/gcc.target/i386/pr32219-8.c | 17 ++ gcc/testsuite/gcc.target/i386/pr64317.c | 2 +- gcc/varasm.c | 95 ++- 13 files changed, 260 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/visibility-22.c create mode 100644 gcc/testsuite/gcc.dg/visibility-23.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-1.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-2.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-3.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-4.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-5.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-6.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-7.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-8.c diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 35b244e..71367a3 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -792,8 +792,10 @@ varpool_node::finalize_decl (tree decl) if (node->definition) return; - notice_global_symbol (decl); + /* Set definition first before calling notice_global_symbol so that + it is available to notice_global_symbol. */ node->definition = true; + notice_global_symbol (decl); if (TREE_THIS_VOLATILE (decl) || DECL_PRESERVE_P (decl) /* Traditionally we do not eliminate static variables when not optimizing and when not doing to
Re: [PATCH] PR rtl-optimization/32219: optimizer causes wrong code in pic/hidden/weak symbol checking
On Sat, Feb 07, 2015 at 07:56:06AM -0800, H.J. Lu wrote: > On Sat, Feb 07, 2015 at 10:11:01AM -0500, Jack Howarth wrote: > > H.J,, > > Unfortunately, the answer is yes. This patch still introduces > > regressions in the g++ test suite.l These are all some form of... > > > > It looks like Darwin depends on the old behavior of > default_binds_local_p_1. Please try this patch. > Here is an updated patch. H.J. >From d010cd1ddf866f8e10fe7ad2cf483b5a872bc6ea Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Thu, 5 Feb 2015 14:28:58 -0800 Subject: [PATCH] Handle symbol visibility/locality for PIE/PIC If a hidden weak symbol isn't defined in the TU, we can't assume it will be defined in another TU at link time. It makes a difference in code generation when compiling for PIC. If we assume that a hidden weak undefined symbol is local, the address checking may be optimized out and leads to the wrong code. This means that a symbol with user specified visibility is local only if it is locally resolved or defined, not weak or not compiling for PIC. When symbol visibility is specified in the source, we should always output symbol visibility even if symbol isn't local to the TU. If a global data symbol is defined in the TU, it is always local to the executable, regardless if it is a common symbol or not. If we aren't compiling for shared library, locally defined global data symbol binds locally. Since some targets call default_binds_local_p_1 directly and depend on the old behavior of default_binds_local_p_1. This patch renames default_binds_local_p_1 to default_binds_local_p_2 and implements the new behavior in default_binds_local_p_2 controlled by a variable. The old behavior remains with default_binds_local_p_1. gcc/ PR rtl-optimization/32219 * cgraphunit.c (varpool_node::finalize_decl): Set definition first before calling notice_global_symbol so that it is available to notice_global_symbol. * varasm.c (default_binds_local_p_1): Renamed to ... (default_binds_local_p_2): This. Resolve defined data symbol locally if not building shared library. Resolve symbol with user specified visibility locally only if it is locally resolved or defined, not weak or not compiling for PIC. (default_binds_local_p): Replace default_binds_local_p_1 with default_binds_local_p_2. (default_binds_local_p_1): Call default_binds_local_p_2. (default_elf_asm_output_external): Always output visibility specified in the source. gcc/testsuite/ PR rtl-optimization/32219 * gcc.dg/visibility-22.c: New test. * gcc.dg/visibility-23.c: Likewise. * gcc.target/i386/pr32219-1.c: Likewise. * gcc.target/i386/pr32219-2.c: Likewise. * gcc.target/i386/pr32219-3.c: Likewise. * gcc.target/i386/pr32219-4.c: Likewise. * gcc.target/i386/pr32219-5.c: Likewise. * gcc.target/i386/pr32219-6.c: Likewise. * gcc.target/i386/pr32219-7.c: Likewise. * gcc.target/i386/pr32219-8.c: Likewise. * gcc.target/i386/pr64317.c: Expect GOTOFF relocation instead of GOT relocation. --- gcc/cgraphunit.c | 4 +- gcc/testsuite/gcc.dg/visibility-22.c | 17 + gcc/testsuite/gcc.dg/visibility-23.c | 15 gcc/testsuite/gcc.target/i386/pr32219-1.c | 16 gcc/testsuite/gcc.target/i386/pr32219-2.c | 16 gcc/testsuite/gcc.target/i386/pr32219-3.c | 17 + gcc/testsuite/gcc.target/i386/pr32219-4.c | 17 + gcc/testsuite/gcc.target/i386/pr32219-5.c | 16 gcc/testsuite/gcc.target/i386/pr32219-6.c | 16 gcc/testsuite/gcc.target/i386/pr32219-7.c | 17 + gcc/testsuite/gcc.target/i386/pr32219-8.c | 17 + gcc/testsuite/gcc.target/i386/pr64317.c | 2 +- gcc/varasm.c | 61 +-- 13 files changed, 209 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/visibility-22.c create mode 100644 gcc/testsuite/gcc.dg/visibility-23.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-1.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-2.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-3.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-4.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-5.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-6.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-7.c create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-8.c diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 35b244e..71367a3 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -792,8 +792,10 @@ varpool_node::finalize_decl (tree decl) if (node->definition) return; - notice_global_symbol (decl); + /* Set definition first before calling notice_global_symbol so that + it is available to notice_global_symbol. */ no
Re: [PATCH] PR rtl-optimization/32219: optimizer causes wrong code in pic/hidden/weak symbol checking
H.J., This one bootstrap and appears to give clean c++ test suite results so far on x86_64-apple-darwin14. I am stopping the regression testing to try the updated patch you sent later. Jack On Sat, Feb 7, 2015 at 10:56 AM, H.J. Lu wrote: > On Sat, Feb 07, 2015 at 10:11:01AM -0500, Jack Howarth wrote: >> H.J,, >> Unfortunately, the answer is yes. This patch still introduces >> regressions in the g++ test suite.l These are all some form of... >> > > It looks like Darwin depends on the old behavior of > default_binds_local_p_1. Please try this patch. > > > H.J. > > From 2df2188c97760ed10a85bff3dfef8198e41862f2 Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" > Date: Thu, 5 Feb 2015 14:28:58 -0800 > Subject: [PATCH] Handle symbol visibility/locality for PIE/PIC > > If a hidden weak symbol isn't defined in the TU, we can't assume it will > be defined in another TU at link time. It makes a difference in code > generation when compiling for PIC. If we assume that a hidden weak > undefined symbol is local, the address checking may be optimized out and > leads to the wrong code. This means that a symbol with user specified > visibility is local only if it is locally resolved or defined, not weak > or not compiling for PIC. When symbol visibility is specified in the > source, we should always output symbol visibility even if symbol isn't > local to the TU. > > If a global data symbol is defined in the TU, it is always local to the > executable, regardless if it is a common symbol or not. If we aren't > compiling for shared library, locally defined global data symbol binds > locally. > > Since some targets call default_binds_local_p_1 directly and depend on > the old behavior of default_binds_local_p_1. This patch copies > default_binds_local_p_1 to default_binds_local_p_2 and changes the > behavior of default_binds_local_p by calling default_binds_local_p_2 > instead of default_binds_local_p_1. > > gcc/ > > PR rtl-optimization/32219 > * cgraphunit.c (varpool_node::finalize_decl): Set definition > first before calling notice_global_symbol so that it is > available to notice_global_symbol. > * varasm.c (default_binds_local_p_2): Copied from > default_binds_local_p_1. Resolve defined data symbol locally if > not building shared library. Resolve symbol with user specified > visibility locally only if it is locally resolved or defined, not > weak or not compiling for PIC. > (default_binds_local_p): Replace default_binds_local_p_1 with > default_binds_local_p_2. > (default_elf_asm_output_external): Always output visibility > specified in the source. > > gcc/testsuite/ > > PR rtl-optimization/32219 > * gcc.dg/visibility-22.c: New test. > * gcc.dg/visibility-23.c: Likewise. > * gcc.target/i386/pr32219-1.c: Likewise. > * gcc.target/i386/pr32219-2.c: Likewise. > * gcc.target/i386/pr32219-3.c: Likewise. > * gcc.target/i386/pr32219-4.c: Likewise. > * gcc.target/i386/pr32219-5.c: Likewise. > * gcc.target/i386/pr32219-6.c: Likewise. > * gcc.target/i386/pr32219-7.c: Likewise. > * gcc.target/i386/pr32219-8.c: Likewise. > * gcc.target/i386/pr64317.c: Expect GOTOFF relocation instead > of GOT relocation. > --- > gcc/cgraphunit.c | 4 +- > gcc/testsuite/gcc.dg/visibility-22.c | 17 ++ > gcc/testsuite/gcc.dg/visibility-23.c | 15 + > gcc/testsuite/gcc.target/i386/pr32219-1.c | 16 ++ > gcc/testsuite/gcc.target/i386/pr32219-2.c | 16 ++ > gcc/testsuite/gcc.target/i386/pr32219-3.c | 17 ++ > gcc/testsuite/gcc.target/i386/pr32219-4.c | 17 ++ > gcc/testsuite/gcc.target/i386/pr32219-5.c | 16 ++ > gcc/testsuite/gcc.target/i386/pr32219-6.c | 16 ++ > gcc/testsuite/gcc.target/i386/pr32219-7.c | 17 ++ > gcc/testsuite/gcc.target/i386/pr32219-8.c | 17 ++ > gcc/testsuite/gcc.target/i386/pr64317.c | 2 +- > gcc/varasm.c | 95 > ++- > 13 files changed, 260 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/visibility-22.c > create mode 100644 gcc/testsuite/gcc.dg/visibility-23.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-1.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-2.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-3.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-4.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-5.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-6.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-7.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-8.c > > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > index 35b244e..71367a3 100644 > --- a/gcc/cgraphunit.c > +++ b/gcc/cgraphunit.c > @@ -792,8 +792,10 @@ varpool_no
Re: [patch, doc] copy-edit inline asm sections
On 02/05/2015 11:34 PM, David Wohlferd wrote: On 2/5/2015 9:13 AM, Sandra Loosemore wrote: In addition to fixing markup and capitalization, I've moved things around a little bit to improve the flow, and rephrased a few things that I thought were awkward or confusing. I propose to commit this in a few days unless somebody tells me meanwhile that I've broken something there's no doubt a lot more that could be done to improve this section further, but I hope this is at least an incremental improvement. Looks like some good changes here. I do have a few suggestions: [snip] Thanks for the careful review! I've committed the attached version of the patch that fixes most of the things you've pointed out (in some cases with different wording than you suggested), except for these: 6) (Symbol-Renaming Pragmas) Put back the text as Rainer Orth requested: -assembly for a given declaration. This effect can also be achieved -using the asm labels extension (@pxref{Asm Labels}). +assembly for a given declaration. While this pragma is supported on all +platforms, it is intended primarily to provide compatibility with the +Solaris system headers. This effect can also be achieved using the asm +labels extension (@pxref{Asm Labels}). 7) (Loop-Specific Pragmas) Grammar error. -dependencies which would prevent that consecutive iterations of -the following loop can be executed concurrently with SIMD +dependencies which would prevent consecutive iterations of +the following loop from executing concurrently with SIMD 8) In invoke.texi (i386 and x86-64 Options) List -masm dialects in dialect order. -Output assembly instructions using selected @var{dialect}. Supported -choices are @samp{intel} or @samp{att} (the default). Darwin does +Output assembly instructions using selected @var{dialect}. Also affects +which dialect is used for Basic @code{asm} (@pxref{Basic Asm}) and +Extended @code{asm} (@pxref{Extended Asm}). Supported choices (in Dialect +order) are @samp{att} or @samp{intel}. The default is @samp{att}. Darwin does Since these are fixes to different parts of the manual, lumping them in with this patch seemed like mission creep to me. Would you like to submit these patches yourself? The first one looks plausible but is outside my area of expertise, the second one is obvious and could just be committed directly, and the third one looks ok except that "Basic", "Extended", and "Dialect" should not be capitalized in the running text. -Sandra 2015-02-07 Sandra Loosemore gcc/ * doc/extend.texi (Function Attributes [naked]): Copy-edit. (Using Assembly Language with C): Expand introduction. (Basic Asm): Copy-edit. Add more information about uses of basic asm. (Extended Asm): Copy-edit. Document new escape syntax and %l[label] syntax. (Global Reg Vars): Copy-edit. (Local Reg Vars): Likewise. Index: gcc/doc/extend.texi === --- gcc/doc/extend.texi (revision 220505) +++ gcc/doc/extend.texi (working copy) @@ -3396,10 +3396,10 @@ This attribute is available on the ARM, RL78, RX and SPU ports. It allows the compiler to construct the requisite function declaration, while allowing the body of the function to be assembly code. The specified function will not have -prologue/epilogue sequences generated by the compiler. Only Basic +prologue/epilogue sequences generated by the compiler. Only basic @code{asm} statements can safely be included in naked functions -(@pxref{Basic Asm}). While using Extended @code{asm} or a mixture of -Basic @code{asm} and ``C'' code may appear to work, they cannot be +(@pxref{Basic Asm}). While using extended @code{asm} or a mixture of +basic @code{asm} and C code may appear to work, they cannot be depended upon to work reliably and are not supported. @item near @@ -6382,12 +6382,25 @@ access hardware. @node Using Assembly Language with C @section How to Use Inline Assembly Language in C Code +@cindex @code{asm} keyword +@cindex assembly language in C +@cindex inline assembly language +@cindex mixing assembly language and C + +The @code{asm} keyword allows you to embed assembler instructions +within C code. GCC provides two forms of inline @code{asm} +statements. A @dfn{basic @code{asm}} statement is one with no +operands (@pxref{Basic Asm}), while an @dfn{extended @code{asm}} +statement (@pxref{Extended Asm}) includes one or more operands. +The extended form is preferred for mixing C and assembly language +within a function, but to include assembly language at +top level you must use basic @code{asm}. -GCC provides various extensions that allow you to embed assembler within -C code. +You can also use the @code{asm} keyword to override the assembler name +for a C symbol, or to place a C variable in a specific register. @menu -* Basic Asm:: Inline assembler with no operands. +* Basic Asm:: Inline
Re: [PATCH/AARCH64] Fix 64893: ICE with vget_lane_u32 with C++ front-end at -O0
On Fri, Feb 6, 2015 at 5:02 PM, Andrew Pinski wrote: > On Mon, Feb 2, 2015 at 11:37 PM, Jakub Jelinek wrote: >> On Mon, Feb 02, 2015 at 02:51:43PM -0800, Andrew Pinski wrote: >>> While trying to build the GCC 5 with GCC 5, I ran into an ICE when >>> building libcpp at -O0. The problem is the C++ front-end was not >>> folding sizeof(a)/sizeof(a[0]) when passed to a function at -O0. The >>> C++ front-end keeps around sizeof until the gimplifier and there is no >>> way to fold the expressions that involve them. So to work around the >>> issue we need to change __builtin_aarch64_im_lane_boundsi to accept an >>> extra argument and change the first two arguments to size_t type so we >>> don't get an extra cast there and do the division inside the compiler >>> itself. >> >> Relying on anything being folded at -O0 when the language does not guarantee >> it is going to be more and more of a problem. So I think your patch is >> reasonable (of course, I'll defer this to target maintainers). >> >>> + rtx totalsize = expand_normal (CALL_EXPR_ARG (exp, 0)); >>> + rtx elementsize = expand_normal (CALL_EXPR_ARG (exp, 1)); >>> + if (CONST_INT_P (totalsize) && CONST_INT_P (elementsize)) >>> + { >>> + rtx lane_idx = expand_normal (CALL_EXPR_ARG (exp, 2)); >>> + if (CONST_INT_P (lane_idx)) >>> + aarch64_simd_lane_bounds (lane_idx, 0, UINTVAL >>> (totalsize)/UINTVAL (elementsize), exp); >> >> Too long line? Also, missing spaces around / . And, ICE if >> somebody uses __builtin_aarch64_im_lane_boundsi (4, 0, 0); >> So you need to check and complain for zero elementsize too. > > Good points, I don't know why I missed this. > >> >>> + else >>> + error ("%Klane index must be a constant immediate", exp); >>> + } >>>else >>> - error ("%Klane index must be a constant immediate", exp); >>> + sorry ("%Ktotal size and element size must be a constant immediate", >>> exp); >> >> But why sorry? If you say the builtin requires constant arguments, then it >> is not sorry, but error, it is not an unimplemented feature. > > Because I originally thought that would be better than error but now > thinking this over, I was incorrect, it should be an error. > I will add a testcase for the __builtin_aarch64_im_lane_boundsi (4, 0, > 0) case and retest the patch. Here is the updated patch with Jakub's comments included and added a testcase for the 0, 0 case. Thanks, Andrew Pinski ChangeLog: PR target/64893 * config/aarch64/aarch64-builtins.c (aarch64_init_simd_builtins): Change the first argument type to size_type_node and add another size_type_node. (aarch64_simd_expand_builtin): Handle the new argument to AARCH64_SIMD_BUILTIN_LANE_CHECK and don't ICE but rather print an out when the first two arguments are not nonzero integer constants. * config/aarch64/arm_neon.h (__AARCH64_LANE_CHECK): Pass the sizeof directly to __builtin_aarch64_im_lane_boundsi. testsuite/ChangeLog: * c-c++-common/torture/aarch64-vect-lane-1.c: New testcase. * c-c++-common/torture/aarch64-vect-lane-2.c: New testcase. > > Thanks, > Andrew > > >> >> Jakub commit b5c809bddf8a34f490ee0cd540f12be1b8b2b897 Author: Andrew Pinski Date: Mon Feb 2 18:40:08 2015 + Fix bug 64893: ICE with vget_lane_u32 with C++ front-end PR target/64893 * config/aarch64/aarch64-builtins.c (aarch64_init_simd_builtins): Change the first argument type to size_type_node and add another size_type_node. (aarch64_simd_expand_builtin): Handle the new argument to AARCH64_SIMD_BUILTIN_LANE_CHECK and don't ICE but rather print an out when the first two arguments are not nonzero integer constants. * config/aarch64/arm_neon.h (__AARCH64_LANE_CHECK): Pass the sizeof directly to __builtin_aarch64_im_lane_boundsi. * testsuite/c-c++-common/torture/aarch64-vect-lane-1.c: New testcase. * testsuite/c-c++-common/torture/aarch64-vect-lane-2.c: New testcase. diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c index 87f1ac2..eabf873 100644 --- a/gcc/config/aarch64/aarch64-builtins.c +++ b/gcc/config/aarch64/aarch64-builtins.c @@ -712,7 +712,8 @@ aarch64_init_simd_builtins (void) aarch64_init_simd_builtin_scalar_types (); tree lane_check_fpr = build_function_type_list (void_type_node, - intSI_type_node, + size_type_node, + size_type_node, intSI_type_node, NULL); aarch64_builtin_decls[AARCH64_SIMD_BUILTIN_LANE_CHECK] = @@ -1000,14 +1001,24 @@ rt
Re: [PATCH, AArch64] Handle SYMBOL_SMALL_TPREL appropriately
On Sun, Feb 1, 2015 at 8:51 PM, Hurugalawadi, Naveen wrote: > Hi, > > Please find attached the patch that handles the operations on > SYMBOL_SMALL_TPREL appropriately. > It fixes gcc.dg/tls/opt-11.c regression on ilp32. > > Please review the patch and let us know if its okay? > Regression tested on aarch64-elf. Also bootstrapped and tested on aarch64-linux-gnu with no regressions (and many testcases in libgomp passing now). Thanks, Andrew Pinski > > Thanks, > Naveen > > 2015-02-02 Andrew Pinski > Naveen H.S > > * config/aarch64/aarch64.c (*aarch64_load_symref_appropriately): > Check whether the destination of SYMBOL_SMALL_TPREL is Pmode.
Re: Stepping down as global maintainer
On 02/06/15 17:42, Diego Novillo wrote: It's been a long time since I did any significant work on GCC, and it is unlikely that I'll be doing much for the foreseeable future. While I still have some understanding of the modules I used to maintain, I don't think it is reasonable to have me making decisions on them. It's been too long and I am not likely to have a good understanding of these components anymore. As such, I propose to become a write-after-approval maintainer and relinquish all the other maintainer roles I had. I'll be committing this to trunk. I kind-of figured something like this would eventually happen. You're always welcome in this community if you ever get the opportunity to re-engage at a higher level. jeff
Re: [RFC][PR target/39726 P4 regression] match.pd pattern to do type narrowing
On 02/03/15 05:23, Joseph Myers wrote: On Tue, 3 Feb 2015, Jeff Law wrote: +/* Given a bit-wise operation performed in mode P1 on operands + in some narrower type P2 that feeds an outer masking operation. + See if the mask turns off all the bits outside P2, and if so + perform the all the operations in P2 and just convert the final + result from P1 to P2. */ +(for inner_op (bit_and bit_ior bit_xor) + (simplify +(bit_and (inner_op (convert @0) (convert @1)) INTEGER_CST@3) +(if ((TREE_INT_CST_LOW (@3) & ~GET_MODE_MASK (TYPE_MODE (TREE_TYPE (@0 == 0 Are you sure about checking TREE_INT_CST_LOW here? What if the inner type is wider than HOST_WIDE_INT? (That could occur with bit-fields of type __int128, for example.) I think the check for what bits are set needs to be written in a wide-int-safe way - maybe something like tree_int_cst_sgn (@3) > 0 && tree_int_cst_min_precision (@3, UNSIGNED) <= TYPE_PRECISION (TREE_TYPE (@1)). It's a WIP :-) +&& TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (TREE_TYPE (@1)) +&& TYPE_UNSIGNED (TREE_TYPE (@0)) == TYPE_UNSIGNED (TREE_TYPE (@1)) +&& TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0))) + (convert (bit_and (inner_op @0 @1) (convert @3)) I still don't think this is safe. Suppose @0 and @1 are -128 in type int8_t and @3 is 128 in a wider type and the operation is AND. Then the original expression has result 128. But if you convert @3 to int8_t you get -128 and this would result in -128 from the simplified expression. Agreed. If the inner values are signed and the mask includes the sign bit of the inner type, you have to zero-extend to the wider type (e.g. convert the inner values to unsigned), not sign-extend. FWIW I did figure out how to get at an expression's type (you can capture them just like individual operands). This is important because if we introduce that signed->unsigned conversions, we have to avoid infinite recursion of the pattern and the easiest way is actually to look at the various types. (If the inner values are signed, it's *also* valid to optimize with a mask where both the sign bit of the inner type and all higher bits are set, such as a mask of -128 above; in that case, you do need to sign-extend. If the inner values are unsigned, no check on the mask value is needed at all as all higher bits in the mask can just be discarded. Both of these statements only apply for bitwise operations, not arithmetic.) Noted. Thanks. I'm going to go through another iteration on this stuff. I'm less concerned about this particular PR, but the knowledge gained here looks to be helpful for 45397 which looks like it might be solveable with match.pd patterns too. jeff