Re: [PATCH 0/2] Introduce a new GCC option, --record-gcc-command-line
Hi Egeyar, Thanks for including me in this discussion. >>> This option is similar to -frecord-gcc-switches. For the record I will also note that there is -fverbose-asm which does almost the same thing, but only records the options as comments in the assembler. They are never converted into data in the actual object files. It is also worth noting that if your goal is to record how a binary was produced, possibly with an eye to reproducibility, then you may also need to record some environment variables too. One thing I found with annobin is that capturing preprocessor options (eg -D_FORTIFY_SOURCE) can be quite hard from inside gcc, since often they have already been processed and discarded. I do not know if this affects your actual patch though. Speaking of annobin, I will bang the gcc plugin gong again here and say that if your patch is rejected then you might want to consider turning it into a plugin instead. In that way you will not need approval from the gcc maintainers. But of course you will have to maintain and publicise the plugin yourself. One other thought occurs to me, which is that if the patch is acceptable, or at least the idea of it, then maybe it would be better to amalgamate all of the current command line recording options into a single version. Eg: --frecord-options=[dwarf,assembler,object] where: --frecord-options=dwarf is a synonym for -grecord-switches --frecord-options=assembler is a synonym for -fverbose-asm --frecord-options=object is a synonym for your option The user could supply one or more of the selectors to have the recording happen in multiple places. Just an idea. Cheers Nick
RFA: Fix libiberty testsuite failure
Hi Ian, The libiberty testsuite in the gcc mainline is currently failing on the last test: FAIL at line 1452, options : in: _Z3fooILPv0EEvPN9enable_ifIXeqT_LDnEEvE4typeE out: void foo<(void*)0>(enable_if<((void*)0)==(decltype(nullptr)), void>::type*) exp: void foo<(void*)0>(enable_if<((void*)0)==((decltype(nullptr))), void>::type*) To me it looks like the expected demangling is incorrect - it wants a double set of parentheses around decltype(nullptr) when I think that only one is needed. So I would like to apply the patch below to fix this. Is this OK ? Cheers Nick libiberty/ChangeLog 2020-01-20 Nick Clifton * testsuite/demangle-expected: Fix expected demangling. Index: libiberty/testsuite/demangle-expected === --- libiberty/testsuite/demangle-expected (revision 280157) +++ libiberty/testsuite/demangle-expected (working copy) @@ -1449,4 +1449,4 @@ #PR91979 demangling nullptr expression _Z3fooILPv0EEvPN9enable_ifIXeqT_LDnEEvE4typeE -void foo<(void*)0>(enable_if<((void*)0)==((decltype(nullptr))), void>::type*) +void foo<(void*)0>(enable_if<((void*)0)==(decltype(nullptr)), void>::type*)
Re: [PATCH] wwwdocs: contribute.html: Update consensus on patch content.
Hi Christophe, I have a follow-up one: I think the same applies to binutils, but I don't think any maintainer / contributor expressed an opinion, and IIUC patch policy for binutils is (lightly) documented at https://sourceware.org/binutils/wiki/HowToContribute Maybe Nick can update it? Done. (I don't have such rights) Would you like them ? It is easy enough to set up. Cheers Nick
Re: [PATCH 14/52] fr30: Remove macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Hi Kewen, This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE defines in fr30 port. gcc/ChangeLog: * config/fr30/fr30.h (FLOAT_TYPE_SIZE): Remove. (DOUBLE_TYPE_SIZE): Likewise. (LONG_DOUBLE_TYPE_SIZE): Likewise. Approved - please apply. Cheers Nick
Re: [PATCH 15/52] frv: Remove macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Hi Kewen, This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE defines in frv port. gcc/ChangeLog: * config/frv/frv.h (FLOAT_TYPE_SIZE): Remove. (DOUBLE_TYPE_SIZE): Likewise. (LONG_DOUBLE_TYPE_SIZE): Likewise. Approved - please apply. Cheers Nick
Re: [PATCH 18/52] iq2000: Remove macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Hi Kewen, This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE defines in iq2000 port. gcc/ChangeLog: * config/iq2000/iq2000.h (FLOAT_TYPE_SIZE): Remove. (DOUBLE_TYPE_SIZE): Likewise. (LONG_DOUBLE_TYPE_SIZE): Likewise. Approved - please apply. Cheers Nick
Re: [PATCH 20/52] m32c: Remove macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Hi Kewen, This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE defines in m32c port. gcc/ChangeLog: * config/m32c/m32c.h (FLOAT_TYPE_SIZE): Remove. (DOUBLE_TYPE_SIZE): Likewise. (LONG_DOUBLE_TYPE_SIZE): Likewise. Approved - please apply. Cheers Nick
Re: [PATCH 21/52] m32r: Remove macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Hi Kewen, This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE defines in m32r port. gcc/ChangeLog: * config/m32r/m32r.h (FLOAT_TYPE_SIZE): Remove. (DOUBLE_TYPE_SIZE): Likewise. (LONG_DOUBLE_TYPE_SIZE): Likewise. Approved - please apply. Cheers Nick
Re: [PATCH 25/52] msp430: Remove macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Hi Kewen, This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE defines in msp430 port. gcc/ChangeLog: * config/msp430/msp430.h (FLOAT_TYPE_SIZE): Remove. (DOUBLE_TYPE_SIZE): Likewise. (LONG_DOUBLE_TYPE_SIZE): Likewise. Approved - please apply. Cheers Nick
Re: [PATCH 32/52] stormy16: Remove macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Hi Kewen, This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE defines in stormy16 port. gcc/ChangeLog: * config/stormy16/stormy16.h (FLOAT_TYPE_SIZE): Remove. (DOUBLE_TYPE_SIZE): Likewise. (LONG_DOUBLE_TYPE_SIZE): Likewise. Approved - please apply. Cheers Nick
Re: [PATCH 43/52] rx: New hook implementation rx_c_mode_for_floating_type
Hi Kewen, This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE defines in rx port, and add new port specific hook implementation rx_c_mode_for_floating_type. gcc/ChangeLog: * config/rx/rx.cc (rx_c_mode_for_floating_type): New function. (TARGET_C_MODE_FOR_FLOATING_TYPE): New macro. * config/rx/rx.h (FLOAT_TYPE_SIZE): Remove. (DOUBLE_TYPE_SIZE): Likewise. (LONG_DOUBLE_TYPE_SIZE): Likewise. Approved - please apply. Cheers Nick
Re: [PATCH] RISC-V: Minimal support for Zimop extension.
Hi Jeff, 2.43 was released over an weekend. Is it possible to let it be supported after 2.44? cc Nick and jan. I don't think it's critical enough to backport to 2.43. I'd just put it on the trunk so that it's available in 2.44. It might be worth adding it to the 2.43 branch as well. It is looking like there will be need to create a point release this time as several other last-minute problems have been uncovered and fixed just too late to make it into the 2.43 release. Cheers Nick
Re: [PATCH] RISC-V: Minimal support for Zimop extension.
Hi Nelson, Sounds good to me, too. Once get the approval, I will backport them to binutils-2_43-branch :-) Please could you ping me once you have done that. I will make sure not to make the point release before receiving your message. Cheers Nick
Re: RFA: Fix declaration of default value of TARGET_NARROW_VOLATILE_BITFIELD
Hi Gerald, it looks this patch did not get applied. Is this because you failed to get approval? Yes. :-( With a patch Joseph did recently, this would now be in gcc/target.def, and from what I can see both parts of the patch are fine. Unless anyone objects, can you please ago ahead and commit an update patch? Thanks. I have gone ahead and checked in this patch: Cheers Nick gcc/ChangeLog 2013-08-20 Nick Clifton * target.def (narrow_volatile_bitfield): Note that the default value is false, not !TARGET_STRICT_ALIGN. * doc/tm.texi: Regenerate. Index: target.def === --- target.def (revision 201870) +++ target.def (working copy) @@ -1967,7 +1967,7 @@ should use the narrowest mode possible. It should return @code{false} if\n\ these accesses should use the bitfield container type.\n\ \n\ -The default is @code{!TARGET_STRICT_ALIGN}.", +The default is @code{false}.", bool, (void), hook_bool_void_false)
Re: RFA: AVR: Support building AVR Linux targets
Hi Joseph, Your patch itself makes sense on general principles, but the concept of an AVR Linux target doesn't - this is an 8-bit processor Really, the bug you've found is that there's an avr-*-* case that is too general, matching nonsensical targets such as AVR Linux rather than just avr-*-none / avr-*-elf. Fair enough. Would you like me to A. Apply the patch anyway. On the general principle that target specific sections in config.gcc add to tmake_file rather than override it. B. Do nothing. On the general principle that if it ain't broke don't fix it. C. Draw up another patch that restricts the AVR patterns in config.gcc to -none and -elf. Cheers Nick
Re: RFA: AVR: Support building AVR Linux targets
Hi Joseph, C. Draw up another patch that restricts the AVR patterns in config.gcc to -none and -elf. A and C - I think both changes should be applied. OK - the patch for item A is already applied. Here is a proposed patch for item C. I have not applied the patch as obvious because I was not sure whether it was better to accept avr-*-elf or avr-*-elf*. I went for the latter in case there were AVR ELF variants, but I defer to your superior knowledge. OK to apply ? Cheers Nick gcc/ChangeLog 2013-08-27 Nick Clifton * config.gcc (AVR): Restrict allowed targets to RTEMS, ELF and NONE. Index: gcc/config.gcc === --- gcc/config.gcc (revision 202021) +++ gcc/config.gcc (working copy) @@ -995,7 +995,7 @@ extra_gcc_objs="driver-avr.o avr-devices.o" extra_objs="avr-devices.o avr-log.o" ;; -avr-*-*) +avr-*-elf* | avr-*-none) tm_file="elfos.h avr/elf.h avr/avr-arch.h avr/avr.h dbxelf.h avr/avr-stdint.h" if test x${with_avrlibc} != xno; then tm_file="${tm_file} ${cpu_type}/avrlibc.h"
Re: [PING 3] [Patch RX] Add assembler option "-mcu" for generating assembler
Hi Sandeep, gas/config: 2013-07-18 Sandeep Kumar Singh * rx.h: Add option -mcpu for target variants RX100 and RX200. Approved - please apply. Cheers Nick
RFA: Testsuite: Add exceptions for MSP430
Hi Guys, The new MSP430 target has a few limitations which show up as unexpected failures in the GCC testsuite. I would like to apply the patch below to update target-supports.exp with this information. Applying the patch results in a drop of 133 unexpected failures (per multilib). OK to apply ? Cheers Nick gcc/testsuite/ChangeLog 2013-09-16 Nick Clifton * lib/target-supports.exp (check_effective_target_trampolines): Add MSP430 to the list of targets that do not support trampolines. (check_profiling_available): Add MSP430 to the list of targets that do not support profiling. (check_effective_target_tls_runtime): Add MSP430 to the list of targets that do not support TLS. Index: gcc/testsuite/lib/target-supports.exp === --- gcc/testsuite/lib/target-supports.exp (revision 202612) +++ gcc/testsuite/lib/target-supports.exp (working copy) @@ -435,9 +435,10 @@ return 0 } if { [istarget avr-*-*] +|| [istarget msp430-*-*] || [istarget hppa2.0w-hp-hpux11.23] - || [istarget hppa64-hp-hpux11.23] } { - return 0; +|| [istarget hppa64-hp-hpux11.23] } { + return 0; } return 1 } @@ -535,6 +536,7 @@ || [istarget mmix-*-*] || [istarget mn10300-*-elf*] || [istarget moxie-*-elf*] +|| [istarget msp430-*-*] || [istarget picochip-*-*] || [istarget powerpc-*-eabi*] || [istarget powerpc-*-elf] @@ -652,6 +654,11 @@ # Return 1 if TLS executables can run correctly, 0 otherwise. proc check_effective_target_tls_runtime {} { +# MSP430 runtime does not have TLS support, but just +# running the test below is insufficient to show this. +if { [istarget msp430-*-*] } { + return 0 +} return [check_runtime tls_runtime { __thread int thr = 0; int main (void) { return thr; }
Commit: MSP430: Add support for interrupt handlers
Hi Guys, I am applying the patch below to add support for interrupt handlers to the MSP430 backend. The patch also adds a couple of MSP430 specific builtin functions intended to be used inside interrupt handlers. In addition the patch adds support for naked functions, critical functions (which disable interrupts whilst they execute) and reentrant functions (which disable interrupts but always reenable them upon exit). Tested with no regressions on an msp430-elf toolchain. Cheers Nick gcc/ChangeLog 2013-09-17 Nick Clifton * config/msp430/msp430-protos.h: Add prototypes for new functions. * config/msp430/msp430.c (msp430_preserve_reg_p): Add support for interrupt handlers. (is_attr_func): New function. (msp430_is_interrupt_func): New function. (is_naked_func): New function. (is_reentrant_func): New function. (is_critical_func): New function. (msp430_start_function): Add annotations for function attributes. (msp430_attr): New function. (msp430_attribute_table): New. (msp430_function_section): New function. (TARGET_ASM_FUNCTION_SECTION): Define. (msp430_builtin): New enum. (msp430_init_builtins): New function. (msp430_builtin_devl): New function. (msp430_expand_builtin): New function. (TARGET_INIT_BUILTINS): Define. (TARGET_EXPAND_BUILTINS): Define. (TARGET_BUILTIN_DECL): Define. (msp430_expand_prologue): Add support for naked, interrupt, critical and reentranct functions. (msp430_expand_epilogue): Likewise. (msp430_print_operand): Handle 'O' character. * config/msp430/msp430.h (TARGET_CPU_CPP_BUILTINS): Define NO_TRAMPOLINES. * config/msp430/msp430.md (unspec): Add UNS_DINT, UNS_EINT, UNS_PUSH_INTR, UNS_POP_INTR, UNS_BIC_SR, UNS_BIS_SR. (pushm): Use a 'n' rather than an 'i' contraint. (msp_return): Add generation of the interrupt return instruction. (disable_interrupts): New pattern. (enable_interrupts): New pattern. (push_intr_state): New pattern. (pop_intr_state): New pattern. (bic_SR): New pattern. (bis_SR): New pattern. * doc/extend.texi: Document MSP430 function attributes and builtin functions. msp430.intr.patch.xz Description: application/xz
Re: RFA: Testsuite: Add exceptions for MSP430
Hi Mike, Ok, I assume that the changes to hppa and return 0 are intentional and good… - || [istarget hppa64-hp-hpux11.23] } { - return 0; +|| [istarget hppa64-hp-hpux11.23] } { + return 0; Sorry - yes - they are just whitespace adjustments so that the entries line up. Cheers Nick
Commit: MSP430: Pass -md on to assembler
Hi Guys, I am applying the patch below to add a couple of minor tweaks to the msp430.h header file. The first is to pass the -md command line option to the assembler, to enable the copying of data from ROM to RAM. (This is a code size optimization. The assembler and linker conspire to detect when the .data section is empty and, if it is, the code to perform the copying is omitted from crt0.o. The -md command line option enables this feature). The other tweak is to define ASM_DECLARE_FUNCTION_NAME so that msp430_start_function can provide some annotations describing the function in the assembler output. This is mainly for debugging purposes. Cheers Nick gcc/ChangeLog 2013-09-18 Nick Clifton * config/msp430/msp430.h (ASM_SPEC): Pass -md on to the assembler. (ASM_DECLARE_FUNCTION_NAME): Define. Index: gcc/config/msp430/msp430.h === --- gcc/config/msp430/msp430.h (revision 202680) +++ gcc/config/msp430/msp430.h (working copy) @@ -54,6 +54,7 @@ "%{mmcu=msp430x:-mmcu=msp430X;mmcu=*:-mmcu=%*} " /* Pass the MCU type on to the assembler. */ \ "%{mrelax=-mQ} " /* Pass the relax option on to the assembler. */ \ "%{mlarge:-ml} " /* Tell the assembler if we are building for the LARGE pointer model. */ \ + "%{!msim:-md} %{msim:%{mlarge:-md}}" /* Copy data from ROM to RAM if necessary. */ \ "%{ffunction-sections:-gdwarf-sections}" /* If function sections are being created then create DWARF line number sections as well. */ /* Enable linker section garbage collection by default, unless we @@ -399,3 +400,7 @@ ) #define ACCUMULATE_OUTGOING_ARGS 1 + +#undef ASM_DECLARE_FUNCTION_NAME +#define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL) \ + msp430_start_function ((FILE), (NAME), (DECL))
Commit: MSP430: Code gen improvements
Hi Guys, I am checking in the attached patch in order to fix some code generation problems exposed by running the GCC testsuite for the MSP430. The patch basically adds a definition of TARGET_PRINT_OPERAND_ADDRESS as some asm() statements needs this. It also adds a sign extending PSI to SI conversion pattern for when signed pointer values need to be stored in an SI pair of registers. Cheers Nick gcc/ChangeLog 2013-09-26 Nick Clifton * config/msp430/msp430.c (msp430_expand_epilogue): Fix compile time warning message. (msp430_print_operand_raw): Delete unused letter parameter. (TARGET_PRINT_OPERAND_ADDRESS): Define. (msp430_print_operand_address): New function. (msp430_print_operand): Move address printing code from here to new function. * config/msp430/msp430.md (movsipsi2): Add comment in generated assembler. (zero_extendpsisi2): Likewise. (extendpsisi2): New pattern. (andneghi3): New pattern. msp430.patch.xz Description: application/xz
Re: Commit: MSP430: Pass -md on to assembler
Hi Mike, I must say though, it seems wrong to have to provide a sign-extend pointer pattern when pointers (on the MSP430) are unsigned. Agreed. If we instead ask, is it sane for gcc to ever want to signed extend in this case, the answer appears to be no. Why does it, ptr_mode is SImode, and expand_builtin_next_arg is used to perform the addition in this mode. It 'just' knows that is can be signed extended… and just does it that way. This seems like it is wrong. Index: builtins.c === --- builtins.c (revision 202634) +++ builtins.c (working copy) @@ -4094,7 +4094,7 @@ expand_builtin_next_arg (void) return expand_binop (ptr_mode, add_optab, crtl->args.internal_arg_pointer, crtl->args.arg_offset_rtx, - NULL_RTX, 0, OPTAB_LIB_WIDEN); + NULL_RTX, POINTERS_EXTEND_UNSIGNED > 0, OPTAB_LIB_WIDEN); } /* Make it easier for the backends by protecting the valist argument would fix this problem. If this is done, the unmodified test case then doesn't abort. Arguably, the extension should be done as the port directs. It isn't clear to me why they do not. Ok? OK by me, although I cannot approve that particular patch. I did eventually find some test cases that exercised the sign-extend pointer pattern, so I was able to check the generated code - it worked OK. But I ran into a very strange problem. With your PARTIAL_INT_MODE_NAME patch applied GCC started erroneously eliminating NULL function pointer checks! This was particularly noticeable in libgcc/crtstuff.c where for example: static void __attribute__((used)) frame_dummy (void) { static struct object object; if (__register_frame_info) __register_frame_info (__EH_FRAME_BEGIN__, &object); (this is a simplified version of the real code) ... is compiled as if it had be written as: static void __attribute__((used)) frame_dummy (void) { static struct object object; __register_frame_info (__EH_FRAME_BEGIN__, &object); This only happens for the LARGE model (when pointers are PSImode) but I was baffled as to where it could be happening. Have you come across anything like this ? Cheers Nick
RFA: GCC Testsuite: Annotate compile tests that need at least 32-bit pointers/integers
Hi Guys, Several tests in the gcc.c-torture/compile directory need a target with 32-bit integers and/or 32-bit pointers. The patch below adds "dg-require-effective-target in32plus" to these tests. This fixes ~200 unexpected failures for the MSP430, RL78 and XSTORMY16 targets. Note - I have used "dg-require-effective-target int32plus" in preference to "dg-require-effective-target ptr32plus" even if it would appear that a pointer size test would be more appropriate. This is because the check_effective_target_ptr32_plus test is broken for targets that use PSImode (eg the MSP430 in large mode). On the MSP430 for example a PSImode pointer is 20-bits long, but when it is stored in memory it occupies 32-bits. So "sizeof(void *)" returns 4 but really a pointer cannot hold an entire 32-bit address. Tested with no regressions on msp430-elf, rl78-elf and xstormy16-elf targets. OK to apply ? Cheers Nick gcc/testsuite/ChangeLog 2013-09-30 Nick Clifton * gcc.c-torture/compile/20010327-1.c: Only run the test for int32plus targets. * gcc.c-torture/compile/990617-1.c: Likewise. * gcc.c-torture/compile/calls.c: Likewise. * gcc.c-torture/compile/limits-externdecl.c: Likewise. * gcc.c-torture/compile/pr41181.c: Likewise. * gcc.c-torture/compile/pr55955.c: Likewise. Index: gcc/testsuite/gcc.c-torture/compile/20010327-1.c === --- gcc/testsuite/gcc.c-torture/compile/20010327-1.c(revision 203032) +++ gcc/testsuite/gcc.c-torture/compile/20010327-1.c(working copy) @@ -1,7 +1,4 @@ -/* { dg-skip-if "non-SI pointers" { m32c-*-* } { "*" } { "" } } */ -/* { dg-skip-if "HI mode pointer for avr" { "avr-*-*" } { "*" } { "" } } */ -/* { dg-skip-if "HI mode pointer for pdp11" { "pdp11-*-*" } { "*" } { "" } } */ -/* { dg-skip-if "non-SI pointers for w64" { "x86_64-*-mingw*" } { "*" } { "" } } */ +/* { dg-require-effective-target int32plus } */ /* This testcase tests whether GCC can produce static initialized data that references addresses of size 'unsigned long', even if that's not Index: gcc/testsuite/gcc.c-torture/compile/20020604-1.c === --- gcc/testsuite/gcc.c-torture/compile/20020604-1.c(revision 203032) +++ gcc/testsuite/gcc.c-torture/compile/20020604-1.c(working copy) @@ -1,7 +1,6 @@ /* { dg-do assemble } */ -/* { dg-skip-if "The array is too big" { "avr-*-*" "pdp11-*-*" } { "*" } { "" } } */ +/* { dg-require-effective-target ptr32plus } */ /* { dg-xfail-if "The array too big" { "h8300-*-*" } { "-mno-h" "-mn" } { "" } } */ -/* { dg-skip-if "" { m32c-*-* } { } { } } */ /* PR c/6957 This testcase ICEd at -O2 on IA-32, because Index: gcc/testsuite/gcc.c-torture/compile/20080625-1.c === --- gcc/testsuite/gcc.c-torture/compile/20080625-1.c(revision 203032) +++ gcc/testsuite/gcc.c-torture/compile/20080625-1.c(working copy) @@ -1,4 +1,5 @@ -/* { dg-skip-if "too much data" { "avr-*-*" "m32c-*-*" "pdp11-*-*" } { "*" } { "" } } */ +/* { dg-require-effective-target int32plus } */ + struct peakbufStruct { unsigned int lnum [5000]; int lscan [5000][4000]; Index: gcc/testsuite/gcc.c-torture/compile/990617-1.c === --- gcc/testsuite/gcc.c-torture/compile/990617-1.c (revision 203032) +++ gcc/testsuite/gcc.c-torture/compile/990617-1.c (working copy) @@ -1,7 +1,5 @@ -/* 0x7000 is too large a constant to become a pointer on - xstormy16. */ /* { dg-do assemble } */ -/* { dg-xfail-if "" { xstormy16-*-* } { "*" } { "" } } */ +/* { dg-require-effective-target int32plus } */ int main() { Index: gcc/testsuite/gcc.c-torture/compile/calls.c === --- gcc/testsuite/gcc.c-torture/compile/calls.c (revision 203032) +++ gcc/testsuite/gcc.c-torture/compile/calls.c (working copy) @@ -1,3 +1,4 @@ +/* { dg-require-effective-target int32plus } */ typedef void *(*T)(void); f1 () { Index: gcc/testsuite/gcc.c-torture/compile/limits-externdecl.c === --- gcc/testsuite/gcc.c-torture/compile/limits-externdecl.c (revision 203032) +++ gcc/testsuite/gcc.c-torture/compile/limits-externdecl.c (working copy) @@ -1,3 +1,4 @@ +/* { dg-require-effective-target int32plus } */ /* Inspired by the test case for PR mi
Re: RFA: GCC Testsuite: Annotate compile tests that need at least 32-bit pointers/integers
Hi Mike, It may be reasonable to special case ptr32plus to say no on your platform, from check_effective_target_tls_native, we see code like: you could do something like: proc check_effective_target_ptr32plus { } { # msp430 never really has 32 or more bits in a pointer. if { [istarget msp430-*-*] } { return 0 } return [check_no_compiler_messages ptr32plus object { int dummy[sizeof (void *) >= 4 ? 1 : -1]; }] } Then, you don't have to worry about people adding tests with this predicate and those test cases failing. I don't have a good handle on wether this is better or not, so, I'll let you decide what you think is best. Thanks - that is a good idea. (I am embarrassed that I did not think of it myself). I have checked the patch in with this change added. Cheers Nick
RFC: Using DECL_NO_LIMIT_STACK as a backend specific flag
Hi Guys, Is there a target specific way to add a flag bit to a function decl ? For the RX port I want to be able to mark a function decl for which I have issued a warning message. Unfortunately I could find no easy way to do this other than stealing one of the generic bits (no_limit_stack in this case). Is there a proper way to do this ? I have attached my current implementation to demonstrate the problem. Cheers Nick Index: gcc/config/rx/rx.c === --- gcc/config/rx/rx.c (revision 192034) +++ gcc/config/rx/rx.c (working copy) @@ -1288,6 +1288,24 @@ target_reinit (); } + if (current_is_fast_interrupt && rx_warn_multiple_fast_interrupts) +{ + static tree prev_fast_interrupt = NULL_TREE; + + if (prev_fast_interrupt == NULL_TREE) + prev_fast_interrupt = fndecl; + else if (prev_fast_interrupt != fndecl + && ! DECL_NO_LIMIT_STACK (fndecl)) + { + warning (0, "multiple fast interrupt routines seen: %qE and %qE", + fndecl, prev_fast_interrupt); + /* FIXME: We use the no_limit_stack bit in the tree_function_decl +structure to mark functions for which we have issued this warning. +There probably ought to be another way to do this. */ + DECL_NO_LIMIT_STACK (fndecl) = 1; + } +} + rx_previous_fndecl = fndecl; } Index: gcc/config/rx/rx.opt === --- gcc/config/rx/rx.opt(revision 192034) +++ gcc/config/rx/rx.opt(working copy) @@ -118,3 +118,9 @@ mpid Target Mask(PID) Enables Position-Independent-Data (PID) mode. + +;--- + +mwarn-multiple-fast-interrupts +Target Report Var(rx_warn_multiple_fast_interrupts) Init(1) Warning +Warn when multiple, different, fast interrupt handlers are in the compilation unit. Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 192034) +++ gcc/doc/invoke.texi (working copy) @@ -859,6 +859,7 @@ -mmax-constant-size=@gol -mint-register=@gol -mpid@gol +-mno-warn-multiple-fast-interrupts@gol -msave-acc-in-interrupts} @emph{S/390 and zSeries Options} @@ -17875,6 +17876,15 @@ By default this feature is not enabled. The default can be restored via the @option{-mno-pid} command-line option. +@item -mno-warn-multiple-fast-interrupts +@itemx -mwarn-multiple-fast-interrupts +@opindex mno-warn-multiple-fast-interrupts +@opindex mwarn-multiple-fast-interrupts +Prevents GCC from issuing a warning message if it finds more than one +fast interrupt handler when it is compiling a file. The default is to +issue a warning for each extra fast interrupt handler found, as the RX +only supports one such interrupt. + @end table @emph{Note:} The generic GCC command-line option @option{-ffixed-@var{reg}}
Re: RFC: Using DECL_NO_LIMIT_STACK as a backend specific flag
Hi Ian, Can't you just keep a list of the decls for which you have issued the warning? Yes - that would work too. In fact I agree that this would be cleaner solution in my particular case. I'll create a new patch... How many decls do you expect to be on that list? Not very many. Maybe two or three at most. But I am interested to know if there is a way for targets to add their own information to function decls (and decls in general). If not for this particular case, then for problems to come in the future. Cheers Nick
Commit: RX: Warn about multiple fast interrupt routines.
Hi Guys, I am applying the patch below to the RX backend. It adds support for issuing warning messages when multiple fast interrupt routines are defined in the same compilation unit. The RX only supports one fast interrupt so it is probably a mistake to define more than one. If the programmer really does want to define them then the command line -mno-warn-multiple-fast-interrupts can be used. Of course it is possible to define multiple fast interrupt handlers in multiple compilation units and then link them together, but Renesas were not interested in solving this problem. Cheers Nick PS. I have even remembered to include a patch to the html documentation as well! gcc/ChangeLog 2012-10-03 Nick Clifton * config/rx/rx.c (struct decl_chain): New local structure. (warned_decls): New local variable. Contains a stack of decls for which warnings have been issued. (add_warned_decl): Adds a decl to the stack. (already_warned): Returns true if a given decl is on the stack. (rx_set_current_function): Issue a warning if multiple fast interrupt handlers are defined. * config/rx/rx.opt (mwarn-multiple-fast-interrupts): New option. * doc/invoke.texi: Document the option. Index: gcc/config/rx/rx.c === --- gcc/config/rx/rx.c (revision 192034) +++ gcc/config/rx/rx.c (working copy) @@ -1256,6 +1256,41 @@ } } +struct decl_chain +{ + tree fndecl; + struct decl_chain * next; +}; + +/* Stack of decls for which we have issued warnings. */ +static struct decl_chain * warned_decls = NULL; + +static void +add_warned_decl (tree fndecl) +{ + struct decl_chain * warned = (struct decl_chain *) xmalloc (sizeof * warned); + + warned->fndecl = fndecl; + warned->next = warned_decls; + warned_decls = warned; +} + +/* Returns TRUE if FNDECL is on our list of warned about decls. */ + +static bool +already_warned (tree fndecl) +{ + struct decl_chain * warned; + + for (warned = warned_decls; + warned != NULL; + warned = warned->next) +if (warned->fndecl == fndecl) + return true; + + return false; +} + /* Perform any actions necessary before starting to compile FNDECL. For the RX we use this to make sure that we have the correct set of register masks selected. If FNDECL is NULL then we are @@ -1288,6 +1323,24 @@ target_reinit (); } + if (current_is_fast_interrupt && rx_warn_multiple_fast_interrupts) +{ + /* We do not warn about the first fast interrupt routine that +we see. Instead we just push it onto the stack. */ + if (warned_decls == NULL) + add_warned_decl (fndecl); + + /* Otherwise if this fast interrupt is one for which we have +not already issued a warning, generate one and then push +it onto the stack as well. */ + else if (! already_warned (fndecl)) + { + warning (0, "multiple fast interrupt routines seen: %qE and %qE", + fndecl, warned_decls->fndecl); + add_warned_decl (fndecl); + } +} + rx_previous_fndecl = fndecl; } Index: gcc/config/rx/rx.opt === --- gcc/config/rx/rx.opt(revision 192034) +++ gcc/config/rx/rx.opt(working copy) @@ -118,3 +118,9 @@ mpid Target Mask(PID) Enables Position-Independent-Data (PID) mode. + +;--- + +mwarn-multiple-fast-interrupts +Target Report Var(rx_warn_multiple_fast_interrupts) Init(1) Warning +Warn when multiple, different, fast interrupt handlers are in the compilation unit. Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 192034) +++ gcc/doc/invoke.texi (working copy) @@ -859,6 +859,7 @@ -mmax-constant-size=@gol -mint-register=@gol -mpid@gol +-mno-warn-multiple-fast-interrupts@gol -msave-acc-in-interrupts} @emph{S/390 and zSeries Options} @@ -17875,6 +17876,15 @@ By default this feature is not enabled. The default can be restored via the @option{-mno-pid} command-line option. +@item -mno-warn-multiple-fast-interrupts +@itemx -mwarn-multiple-fast-interrupts +@opindex mno-warn-multiple-fast-interrupts +@opindex mwarn-multiple-fast-interrupts +Prevents GCC from issuing a warning message if it finds more than one +fast interrupt handler when it is compiling a file. The default is to +issue a warning for each extra fast interrupt handler found, as the RX +only supports one such interrupt. + @end table @emph{Note:} The generic GCC command-line option @option{-ffixed-@var{reg}} Index: htdocs/gcc-4.8/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.8/changes.html,v retrieving revision 1.32 diff -u -3 -p -r1.32 changes.html --- htdocs/g
Re: [patch] unbreak iq2000 build a bit
Hi Steven, iq2000 build is broken since at least r162089 (July 2010). I'm going to commit this patch to fix this part of the build problem. Thanks. It still fails later one, but I don't care about that FYI I have this patch installed in my local sources that allows the iq2000 port to build successfully. I have not submitted it upstream yet because I am not sure if it is the correct fix for the problem. Cheers Nick Index: gcc/dwarf2out.c === --- gcc/dwarf2out.c (revision 192527) +++ gcc/dwarf2out.c (working copy) @@ -20092,11 +20092,13 @@ ca_loc->call_arg_loc_note = loc_note; ca_loc->next = NULL; ca_loc->label = last_label; - gcc_assert (prev - && (CALL_P (prev) - || (NONJUMP_INSN_P (prev) - && GET_CODE (PATTERN (prev)) == SEQUENCE - && CALL_P (XVECEXP (PATTERN (prev), 0, 0); + while (prev != NULL_RTX +&& ! CALL_P (prev) +&& ! (NONJUMP_INSN_P (prev) + && GET_CODE (PATTERN (prev)) == SEQUENCE + && CALL_P (XVECEXP (PATTERN (prev), 0, 0 + prev = prev_nonnote_nondebug_insn (prev); + gcc_assert (prev != NULL_RTX); if (!CALL_P (prev)) prev = XVECEXP (PATTERN (prev), 0, 0); ca_loc->tail_call_p = SIBLING_CALL_P (prev);
Re: Top Level GCC change questions
Hi Steve, It looks like there is another patch that has not been checked in to the GCC top level tree but not binutils and a patch in binutils but not GCC. Is there any automation for this or is it still up to each person checking in files to copy stuff over by hand? As far as I know it is still up to individual contributors to keep these files in sync. Cheers Nick
RFA: Change behaviour of %* in spec strings
Hi Guys, I would like to make a change to the way %* behaves in spec strings. Currently whenever %* is encountered the text that was previously matched is substituted, followed by a space. Thus %{foo=*:bar%*baz} would match -foo=4 and insert 'bar4 baz'. I would like to change this so that the space is only inserted if %* is at the end of a spec sequence. (In all of the strings currently built in to gcc this is the case, so my patch will not change current behaviour). The motivation is that I want to construct a pathname based on a command line option using a spec string like this: %{mmcu=*:--script=%*/memory.ld} So that if the user invokes -mmcu=msp430f2617 then gcc will generate: --script=msp430f2617/memory.ld A spec string like this however: %{mmcu=*:--script=%*}/memory.ld would match the current behaviour and generate: --script=msp430f2617 /memory.ld As a secondary feature of the patch I have also updated the documentation to explicitly state when a space will be inserted into the generated text. I have tested the patch and found no regressions using an i686-pc-liunx-gnu toolchain and an msp430-elf toolchain. OK to apply ? Cheers Nick gcc/ChangeLog 2013-10-17 Nick Clifton * gcc.c (do_spec_1): Do not insert a space after a %* substitution unless it is the last part of a spec substring. * doc/invoke.texi (Spec Files): Document space insertion behaviour of %*. Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 203750) +++ gcc/doc/invoke.texi (working copy) @@ -10929,6 +10929,22 @@ for each matching switch, with the @code{%*} replaced by the part of that switch matching the @code{*}. +If @code{%*} appears as the last part of a spec sequence then a space +will added after the end of the last substitution. If there is more +text in the sequence however then a space will not be generated. This +allows the @code{%*} substitution to be used as part of a larger +string. For example, a spec string like this: + +@smallexample +%@{mcu=*:--script=%*/memory.ld@} +@end smallexample + +when matching an option like @code{-mcu=newchip} will produce: + +@smallexample +--script=newchip/memory.ld +@end smallexample + @item %@{.@code{S}:@code{X}@} Substitutes @code{X}, if processing a file with suffix @code{S}. Index: gcc/gcc.c === --- gcc/gcc.c (revision 203750) +++ gcc/gcc.c (working copy) @@ -388,7 +388,8 @@ %2process CC1PLUS_SPEC as a spec. %*substitute the variable part of a matched option. (See below.) Note that each comma in the substituted string is replaced by - a single space. + a single space. A space is appended after the last substitution + unless there is more text in current sequence. %
Commit: testsuite/gcc.dg/20020312-2.c: Update for RL78 and MSP430
Hi Guys, I am applying the patch below as an obvious update to the 20020312-2.c test for the RL78 and MSP430 targets. Neither of them use a specific regiser for PIC support, but the test needs to be explicitly told that. Cheers Nick gcc/testsuite/ChangeLog 2013-10-24 Nick Clifton * gcc.dg/20020312-2.c: No PIC register for RL78 or MSP430. Index: gcc/testsuite/gcc.dg/20020312-2.c === --- gcc/testsuite/gcc.dg/20020312-2.c (revision 204007) +++ gcc/testsuite/gcc.dg/20020312-2.c (working copy) @@ -96,6 +96,10 @@ #endif #elif defined (__aarch64__) /* No pic register -- yet. */ +#elif defined(__RL78__) +/* No pic register. */ +#elif defined(__MSP430__) +/* No pic register. */ #else # error "Modify the test for your target." #endif
RFA: Remove some assumptions from gcc.dg tests
Hi Guys, The patch below includes a selection of improvements for the tests in the gcc.dg part of the gcc testsuite. It basically annotates tests that have some implicit assumptions about the target that is being tested (word size, supported features, etc). I started making this patch originally just for the MSP430, and so I was adding target specific exceptions. Then I realised that the exceptions would apply to other targets too and that it was better to add dg-require-effective-target-foo statements instead. Tested with an msp430-elf toolchain and an i686-pc-linux-gnu toolchain with no regressions and several improvements. OK to apply ? Cheers Nick gcc/testsuite/ChangeLog 2013-10-24 Nick Clifton * c-c++-common/pr57793.c: Add expected error messages for targets with small integers. * gcc.dg/c99-stdint-1.c: Only run on 32-bit plus targets. * gcc.dg/c99-stdint-2.c: Likewise. * gcc.dg/cdce1.c: Likewise. * gcc.dg/fold-overflow-1.c: Likewise. * gcc.dg/utf-cvt.c: Likewise. * gcc.dg/ftrapv-1.c: Only run on targets that support trapping arithmetic. * gcc.dg/ftrapv-2.c: Likewise. * gcc.dg/pr30286.c: Likewise. * gcc.dg/pr19340.c: Only run on targets that support scheduling. * lib/target-supports.exp (check_effective_target_trapping): New proc. Returns true if the target supports trapping arithmetic. Index: gcc/testsuite/c-c++-common/pr57793.c === --- gcc/testsuite/c-c++-common/pr57793.c(revision 204016) +++ gcc/testsuite/c-c++-common/pr57793.c(working copy) @@ -3,8 +3,8 @@ struct A { unsigned a : 1; unsigned b : 1; }; struct B /* { dg-error "type .B. is too large" "" { target { c++ && ilp32 } } } */ { - unsigned char c[0x4000]; - unsigned char d[0x4ff0]; + unsigned char c[0x4000]; /* { dg-error "size of array .c. is too large" "" { target { ! int32plus } } } */ + unsigned char d[0x4ff0];/* { dg-error "size of array .d. is too large" "" { target { ! int32plus } } } */ struct A e; }; /* { dg-error "type .struct B. is too large" "" { target { c && ilp32 } } } */ Index: gcc/testsuite/gcc.dg/c99-stdint-1.c === --- gcc/testsuite/gcc.dg/c99-stdint-1.c (revision 204016) +++ gcc/testsuite/gcc.dg/c99-stdint-1.c (working copy) @@ -9,6 +9,7 @@ version). */ /* { dg-do compile } */ /* { dg-options "-std=iso9899:1999 -pedantic-errors -fhosted" } */ +/* { dg-require-effective-target ptr32plus } */ #include #include @@ -214,7 +215,6 @@ void test_misc_limits (void) { -/* { dg-bogus "size" "ptrdiff is 16bits" { xfail avr-*-* } 56 } */ CHECK_SIGNED_LIMITS_2(__PTRDIFF_TYPE__, PTRDIFF_MIN, PTRDIFF_MAX, -65535L, 65535L); #ifndef SIGNAL_SUPPRESS CHECK_LIMITS_2(sig_atomic_t, SIG_ATOMIC_MIN, SIG_ATOMIC_MAX, -127, 127, 255); Index: gcc/testsuite/gcc.dg/c99-stdint-2.c === --- gcc/testsuite/gcc.dg/c99-stdint-2.c (revision 204016) +++ gcc/testsuite/gcc.dg/c99-stdint-2.c (working copy) @@ -2,7 +2,7 @@ Freestanding version. */ /* { dg-do compile } */ /* { dg-options "-std=iso9899:1999 -pedantic-errors -ffreestanding" } */ -/* { dg-xfail-if "ptrdiff size is 16bits" { avr-*-* } } */ +/* { dg-require-effective-target ptr32plus } */ /* The test is that there are no diagnostics, so just include the hosted version. */ #include "c99-stdint-1.c" Index: gcc/testsuite/gcc.dg/cdce1.c === --- gcc/testsuite/gcc.dg/cdce1.c(revision 204016) +++ gcc/testsuite/gcc.dg/cdce1.c(working copy) @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -lm" } */ -/* { dg-final { scan-tree-dump "cdce1.c:16: note: function call is shrink-wrapped into error conditions\." "cdce" } } */ +/* { dg-require-effective-target int32plus } */ +/* { dg-final { scan-tree-dump "cdce1.c:17: note: function call is shrink-wrapped into error conditions\." "cdce" } } */ /* { dg-final { cleanup-tree-dump "cdce" } } */ /* { dg-require-effective-target large_double } */ Index: gcc/testsuite/gcc.dg/fold-overflow-1.c === --- gcc/testsuite/gcc.dg/fold-overflow-1.c (revision 204016) +++ gcc/testsuite/gcc.dg/fold-overflow-1.c (working copy) @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-skip-if "consts are shorts, not longs" { "m32c-*-*" "avr-*-*" } { "*" } { "" } } */ +/* { dg-require-effe
Re: RFA: MIPS: Fix race condition causing PR 69129
Hi Matthias, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69129 this fixes the bootstrap errors for me, seen in both libgnat and libgfortran. Great - I have gone ahead and checked the patch in. Cheers Nick
RFA: Fix for cygwin/mingw PR 66655
Hi Guys, The patch below is offered as a fix for PR 66655. In testing it appears that the patch does work, and does not break building libstdc++-v3 for cygwin or mingw. (Unlike the earlier version...) Due to my brain being so small, I have already checked the patch in, without receiving proper authorisation. I apologise for this. If the patch does prove suitable and is approved today, then I will leave it in. Otherwise I will revert the change and wait for proper approval. The patch itself is also slightly dubious in that I am not sure if I have all the correct terms in the if-statement. I was going for minimal impact on the current code, so I went for a set of selectors that matched the testcase for PR 66655, but nothing else. In particular I did not check to see if a similar problem exists for methods or virtual functions. My theory was that if does turn out that these kinds of functions can also trigger this kind of bug, then the patch could be extended later. Plus a new bug report is likely to include a new testcase that can be added to the testsuite. So ... OK to apply ? Cheers Nick gcc/ChangeLog 2016-01-26 Nick Clifton PR target/66655 * config/i386/winnt.c (i386_pe_binds_local_p): If a function has been marked as DECL_ONE_ONLY but we do not the means to make it so, then do not allow it to bind locally. Index: gcc/config/i386/winnt.c === --- gcc/config/i386/winnt.c (revision 232784) +++ gcc/config/i386/winnt.c (working copy) @@ -341,6 +341,20 @@ && TREE_PUBLIC (exp) && DECL_EXTERNAL (exp)) return true; + +#ifndef MAKE_DECL_ONE_ONLY + /* PR target/66655: If a function has been marked as DECL_ONE_ONLY + but we do not the means to make it so, then do not allow it to + bind locally. */ + if (DECL_P (exp) + && TREE_CODE (exp) == FUNCTION_DECL + && TREE_PUBLIC (exp) + && DECL_ONE_ONLY (exp) + && ! DECL_EXTERNAL (exp) + && DECL_DECLARED_INLINE_P (exp)) +return false; +#endif + return default_binds_local_p_1 (exp, 0); }
Re: [PATCH][ARM][2/4] Fix operand costing logic for SMUL[TB][TB]
Hi Kyrill, > 2016-01-22 Kyrylo Tkachov > > * config/arm/arm.c (arm_new_rtx_costs, MULT case): Properly extract > the operands of the SIGN_EXTENDs from a SMUL[TB][TB] rtx. Approved - please apply. Cheers Nick
Re: [PATCH][ARM][1/4] PR target/65932: Add testcase
Hi Kyrill, I would like to approve this patch, but cannot, since it is not ARM specific. I think that if you ping the list you may be able to get a response, and it would be nice to see this whole patch series checked in before the gcc 6 branch occurs. Cheers Nick PS. If necessary you could always move the test to gcc.target/arm...
Re: [PATCH][ARM][4/4] Adjust gcc.target/arm/wmul-[123].c tests
> 2016-01-22 Kyrylo Tkachov > > * gcc.target/arm/wmul-3.c: Simplify test to generate just > a single smulbb instruction. > * gcc.target/amr/wmul-1.c: Add -mtune=cortex-a9 to dg-options. > * gcc.target/amr/wmul-2.c: Likewise. Approved - please apply. Cheers Nick
RFC: Fix ARMv3 support
Hi Richard, Hi Ramana, The ARM backend has some problems compiling for the old ARMv3 architecture. See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62254 for an example of this. v3 is very old now, and I am not sure how much interest there is in continuing to support it, but I am trying to help reduce the gcc priority bug list, so here goes... The attached patch fixes the problem, albeit not in a very subtle way. The problem is that arm_reload_[out|in]_hi is being called for a register->register move because the v3 architecture does not support 16-bit register moves. Rather than trace the problem back to the real source and fix it, I chose to just allow the reload functions to generate an SImode register move instead. Probably not the best solution, but it appears to work. The attached patch also includes the test cases derived from PR 62254 and PR 69610 (which is a duplicate of PR 62254). Including all three tests might be overkill, but it seemed like a good idea to be a little bit paranoid, just in case. Whilst testing the patch I also discovered that interworking is enabled by default, which is a problem for v3 code generation, so I added a fix to (silently) disable interworking if the target architecture does not support Thumb instructions. Any comments or criticisms before I apply the patch ? Cheers Nick gcc/ChangeLog 2016-02-16 Nick Clifton PR target/62554 PR target/69610 * config/arm/arm.c (arm_option_override_internal): Disable interworking if the target does not support thumb instructions. (arm_reload_in_hi): Handle the case where a register to register move needs reloading because there is no simple pattern to handle it. (arm_reload_out_hi): Likewise. gcc/testsuite/ChangeLog 2016-02-16 Nick Clifton PR target/62554 PR target/69610 * gcc.target/arm/pr62554.c: New test. * gcc.target/arm/pr69610-1.c: New test. * gcc.target/arm/pr69610-2.c: New test. Index: gcc/config/arm/arm.c === --- gcc/config/arm/arm.c (revision 233443) +++ gcc/config/arm/arm.c (working copy) @@ -2874,6 +2874,14 @@ { arm_override_options_after_change_1 (opts); + if (TARGET_INTERWORK && !ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB)) +{ + /* The default is to enable interworking, so this warning message would + be confusing to users who have just compiled with, eg, -march=armv3. */ + /* warning (0, "ignoring -minterwork because target CPU does not support THUMB"); */ + opts->x_target_flags &= ~MASK_INTERWORK; +} + if (TARGET_THUMB_P (opts->x_target_flags) && !(ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB))) { @@ -15440,6 +15448,17 @@ else /* The slot is out of range, or was dressed up in a SUBREG. */ base = reg_equiv_address (REGNO (ref)); + + /* PR 62554: If there is no equivalent memory location then just move + the value as an SImode register move. This happens when the target + architecure variant does not have an HImode register move. */ + if (base == NULL) + { + gcc_assert (REG_P (operands[0])); + emit_insn (gen_movsi (gen_rtx_SUBREG (SImode, operands[0], 0), +gen_rtx_SUBREG (SImode, ref, 0))); + return; + } } else base = find_replacement (&XEXP (ref, 0)); @@ -15557,6 +15576,17 @@ else /* The slot is out of range, or was dressed up in a SUBREG. */ base = reg_equiv_address (REGNO (ref)); + + /* PR 62554: If there is no equivalent memory location then just move + the value as an SImode register move. This happens when the target + architecure variant does not have an HImode register move. */ + if (base == NULL) + { + gcc_assert (REG_P (outval)); + emit_insn (gen_movsi (gen_rtx_SUBREG (SImode, ref, 0), +gen_rtx_SUBREG (SImode, outval, 0))); + return; + } } else base = find_replacement (&XEXP (ref, 0)); @@ -19619,6 +19649,7 @@ break; case ARM_FT_INTERWORKED: + gcc_assert (arm_arch5 || arm_arch4t); sprintf (instr, "bx%s\t%%|lr", conditional); break; --- /dev/null 2016-02-16 08:27:18.513962320 + +++ gcc/testsuite/gcc.target/arm/pr62254.c 2016-02-16 16:47:30.479378118 + @@ -0,0 +1,50 @@ +/* Check that pre ARMv4 compilation still works. */ +/* { dg-do compile } */ +/* { dg-options "-marm -march=armv3 -O" } */ + +typedef struct +{ + char bits; + short val; +} code; + +union uu +{ + short us; + char b[2]; +}; + +int a, b, c, f, g, h; +code *d; + +code e; + +int +fn1 (void) +{ + char i; + do +if (e.bits) + { + dodist: +f = c; +if (e.bits & 6) + { +++i; +if (g) + do +{ + union uu j; + j.b[1] = a; + h = j.us; +
Commit: MSP430: Update devices list
Hi Guys, I am checking in this patch to update the list of MSP430 devices built in to the msp430 backend. Cheers Nick gcc/ChangeLog 2016-02-17 Nick Clifton * config/msp430/msp430.c (msp430_mcu_data): Sync with data from TI's devices.csv file as of March 2016. Index: gcc/config/msp430/msp430.c === --- gcc/config/msp430/msp430.c (revision 233486) +++ gcc/config/msp430/msp430.c (working copy) @@ -90,10 +90,10 @@ #define TARGET_OPTION_OVERRIDE msp430_option_override /* This is a copy of the same data structure found in gas/config/tc-msp430.c - Also another (sort-of) copy can be found in gcc/config/msp430/devices-msp430.c + Also another (sort-of) copy can be found in gcc/config/msp430/t-msp430 Keep these three structures in sync. The data in this structure has been extracted from the devices.csv file - released by TI, updated as of 8 October 2015. */ + released by TI, updated as of March 2016. */ struct msp430_mcu_data { @@ -519,7 +519,13 @@ { "msp430fg6626",2,8 }, { "msp430fr2032",2,0 }, { "msp430fr2033",2,0 }, + { "msp430fr2310",2,0 }, + { "msp430fr2311",2,0 }, { "msp430fr2433",2,8 }, + { "msp430fr2532",2,8 }, + { "msp430fr2533",2,8 }, + { "msp430fr2632",2,8 }, + { "msp430fr2633",2,8 }, { "msp430fr2xx_4xxgeneric",2,8 }, { "msp430fr4131",2,0 }, { "msp430fr4132",2,0 }, @@ -553,6 +559,8 @@ { "msp430fr5858",2,8 }, { "msp430fr5859",2,8 }, { "msp430fr5867",2,8 }, + { "msp430fr5862",2,8 }, + { "msp430fr5864",2,8 }, { "msp430fr58671",2,8 }, { "msp430fr5868",2,8 }, { "msp430fr5869",2,8 }, @@ -563,6 +571,8 @@ { "msp430fr5888",2,8 }, { "msp430fr5889",2,8 }, { "msp430fr58891",2,8 }, + { "msp430fr5892",2,8 }, + { "msp430fr5894",2,8 }, { "msp430fr5922",2,8 }, { "msp430fr59221",2,8 }, { "msp430fr5947",2,8 }, @@ -572,6 +582,8 @@ { "msp430fr5957",2,8 }, { "msp430fr5958",2,8 }, { "msp430fr5959",2,8 }, + { "msp430fr5962",2,8 }, + { "msp430fr5964",2,8 }, { "msp430fr5967",2,8 }, { "msp430fr5968",2,8 }, { "msp430fr5969",2,8 }, @@ -584,6 +596,8 @@ { "msp430fr5988",2,8 }, { "msp430fr5989",2,8 }, { "msp430fr59891",2,8 }, + { "msp430fr5992",2,8 }, + { "msp430fr5994",2,8 }, { "msp430fr5xx_6xxgeneric",2,8 }, { "msp430fr6820",2,8 }, { "msp430fr6822",2,8 },
Re: [PATCH][ARM] PR target/69161: Don't ignore mode when matching comparison operator in cstore-like patterns
Hi Kyrill, > Ok for trunk? > > 2016-01-29 Kyrylo Tkachov > > PR target/69161 > * config/arm/predicates.md (arm_comparison_operator_mode): > New predicate. > * config/arm/arm.md (*mov_scc): Use arm_comparison_operator_mode > instead of arm_comparison_operator. > (*mov_negscc): Likewise. > (*mov_notscc): Likewise. > * config/arm/thumb2.md (*thumb2_mov_scc): Likewise. > (*thumb2_mov_negscc): Likewise. > (*thumb2_mov_negscc_strict_it): Likewise. > (*thumb2_mov_notscc): Likewise. > (*thumb2_mov_notscc_strict_it): Likewise. Approved - please apply - but ... > diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md > index > c66c31d5c6047aa7decfe7e95d111d5fbf6fb52e..b8f09ab6b109f80abe2df08a8b7f954f521ec1bf > 100644 > --- a/gcc/config/arm/predicates.md > +++ b/gcc/config/arm/predicates.md > @@ -341,6 +341,11 @@ (define_special_predicate "arm_comparison_operator" >(and (match_operand 0 "expandable_comparison_operator") > (match_test "maybe_get_arm_condition_code (op) != ARM_NV"))) > > +;; Likewise, but don't ignore the mode. > +(define_predicate "arm_comparison_operator_mode" Please could you extend the comment here to reference the PR. That way anyone reading this code who wonders why we need to have two versions of the same predicate will be able understand what is happening. Cheers Nick
RFA: Prevent an ICE when redeclaring a static function as weak
Hi Guys, Redefining a previously defined static function as both public and weak triggers an ICE in ipa-visibility.c: internal compiler error: in function_and_variable_visibility, at ipa-visibility.c:518 This bug has been discussed and patch proposed here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49899 This submission is an updated version of that patch, made against the latest gcc sources. It still chooses to generate an error message and disallow the conversion, which I hope is the correct action. OK to apply ? Cheers Nick gcc/ChangeLog 2016-02-17 Nick Clifton PR middle-end/49889 * varasm.c (merge_weak): Generate an error if an attempt is made to convert a non-weak static function into a weak, public function. gcc/testsuite/ChangeLog 2016-02-17 Nick Clifton PR middle-end/49889 * gcc.dg/pr49889.c: New test. Index: gcc/varasm.c === --- gcc/varasm.c(revision 233486) +++ gcc/varasm.c(working copy) @@ -5366,6 +5366,11 @@ gcc_assert (!TREE_USED (olddecl) || !TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (olddecl))); + /* PR 49899: You cannot convert a static function into a weak, public function. */ + if (! TREE_PUBLIC (olddecl) && TREE_PUBLIC (newdecl)) + error ("weak declaration of %q+D being applied to a already " + "existing, static definition", newdecl); + if (TARGET_SUPPORTS_WEAK) { /* We put the NEWDECL on the weak_decls list at some point. --- /dev/null 2016-02-17 08:13:41.436963282 + +++ gcc/testsuite/gcc.dg/pr49899.c 2016-02-17 14:12:41.066733255 + @@ -0,0 +1,3 @@ +static int foo (void) { return 0; } /* { dg-error "weak declaration of 'foo' being applied to a already existing, static definition" } */ +int foo (void) __attribute__((weak)); +
Re: RFC: Fix ARMv3 support
Hi Christophe, > Could you modify your new testcases, such that they call > check_effective_target_arm_arm_ok ? Good idea - done. > I'm just realizing that we currently generate arm_arch_vX_ok > for X >=4 only. Maybe you should also add v3? Possibly. I am not at all sure how important v3 support is any more (or support for any ARM architecture prior to v4t). Do you know of anyone who still needs it ? Cheers Nick
Re: RFA: Prevent an ICE when redeclaring a static function as weak
Hi Jeff, >>Redefining a previously defined static function as both public and >>weak triggers an ICE in ipa-visibility.c: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49899 > Does this fix a regression? How far back am I allowed to go ? :-) The bug as reported in the PR existed in 4.7.0. I suspect, but have not actually tested, that the bug has always been present. Hence I have to admit that it is probably not a regression. > Essentially I'm trying to figure out if we want this for gcc-6 or gcc-7. Technically I guess the patch ought to be for gcc-7. It just feels wrong to have a patch to fix a known bug, but not to be able to apply it because the bug has always existed... Cheers Nick
Re: RFA: Prevent an ICE when redeclaring a static function as weak
Hi Jeff, > My inclination would be to defer to gcc-7. Richi, Jakub or Joseph, as > release managers, would have the final say though. It's OK. I will resubmit after gcc 6 branches. Cheers Nick
Commit: CR16: Add newlib-stdint.h to tm_file.
Hi Guys, I am applying the patch below as an obvious fix for a problem with the CR16 target - the lack of a definition of __INTPTR_TYPE__. This definition is needed by the newlib C library's _intsup.h header in order to correctly calculate the size of integers and pointers. Cheers Nick gcc/ChangeLog 2016-03-01 Nick Clifton * config.gcc (cr16-*-elf): Add newlib-stdint.h to tm_file. Index: gcc/config.gcc === --- gcc/config.gcc (revision 233857) +++ gcc/config.gcc (working copy) @@ -1168,7 +1168,7 @@ use_gcc_stdint=wrap ;; cr16-*-elf) -tm_file="elfos.h ${tm_file}" +tm_file="elfos.h ${tm_file} newlib-stdint.h" tmake_file="${tmake_file} cr16/t-cr16 " use_collect2=no ;;
RFA: MEP: Add newlib-stdint.h to tm_file.
Hi DJ, Currently the MEP target does not define __INTPTR_TYPE__ or __INT32_TYPE__ which is a problem for newlib. (The newlib header file _intsup.h depends upon these macros being defined). Fortunately the fix is easy - add newlib-stdin.h to the tm_file list for MEP. So, is this patch OK to apply ? Tested with an mep-elf toolchain. Cheers Nick gcc/ChangeLog 2016-03-01 Nick Clifton * config.gcc (mep-*-elf): Add newlib-stdint.h to tm_file. Index: gcc/config.gcc === --- gcc/config.gcc (revision 233858) +++ gcc/config.gcc (working copy) @@ -1973,7 +1973,7 @@ inhibit_libc=true ;; mep-*-*) - tm_file="dbxelf.h elfos.h ${tm_file}" + tm_file="dbxelf.h elfos.h ${tm_file} newlib-stdint.h" tmake_file=mep/t-mep c_target_objs="mep-pragma.o" cxx_target_objs="mep-pragma.o"
Re: RFA: MEP: Add newlib-stdint.h to tm_file.
Hi DJ, > Ok. Thanks - patch committed. (I know that you have now deprecated the MeP port, but at least with this patch in place the toolchain can now be built if obsolete targets are enabled). Cheers Nick
RFA: PR 70044: Catch a second call to aarch64_override_options_after_change
Hi Markus, Hi Richard, PR 70044 reports a problem with the AArch64 backend. With LTO enabled the function aarch64_override_options_after_change can be called more than once for the same function. If only leaf frame pointers were being omitted originally, then the first call will set flag_omit_frame_pointer to true. Then the second call will set flag_omit_leaf_frame_pointer to false, thus forcing the frame pointer to always be omitted, contrary to the original settings of these flags. The attached patch fixes this problem by setting flag_omit_frame_pointer to true, but using a special value of 2 to do so. Then when the second call occurs we can detect this case and ensure that we do not set flag_omit_leaf_frame_pointer to false. Tested with no regressions on an aarch64-elf toolchain. OK to apply ? Cheers Nick gcc/ChangeLog 2016-03-04 Nick Clifton PR target/7044 * config/aarch64/aarch64.c (aarch64_override_options_after_change_1): When forcing flag_omit_frame_pointer to be true, use a special value that can be detected if this function is called again, thus preventing flag_omit_leaf_frame_pointer from being forced to be false. Index: gcc/config/aarch64/aarch64.c === --- gcc/config/aarch64/aarch64.c (revision 233960) +++ gcc/config/aarch64/aarch64.c (working copy) @@ -8110,6 +8110,21 @@ static void aarch64_override_options_after_change_1 (struct gcc_options *opts) { + /* The logic here is that if we are disabling all frame pointer generation + then we do not need to disable leaf frame pointer generation as a + separate operation. But if we are *only* disabling leaf frame pointer + generation then we set flag_omit_frame_pointer to true, but in + aarch64_frame_pointer_required we return false only for leaf functions. + + PR 70044: We have to be careful about being called multiple times for the + same function. Once we have decided to set flag_omit_frame_pointer just + so that we can omit leaf frame pointers, we must then not interpret a + second call as meaning that all frame pointer generation should be + omitted. We do this by setting flag_omit_frame_pointer to a special, + non-zero value. */ + if (opts->x_flag_omit_frame_pointer == 2) +opts->x_flag_omit_frame_pointer = 0; + if (opts->x_flag_omit_frame_pointer) opts->x_flag_omit_leaf_frame_pointer = false; else if (opts->x_flag_omit_leaf_frame_pointer)
Commit: MSP430: Add wakeup function attribute
Hi Guys, I have applied the patch below to add support for a wakeup function attribute to the MSP430 backend. The wakeup attribute applies to interrupt functions, and it makes them wake the processor from low power sleep states when the interrupt handler exits. Cheers Nick gcc/ChangeLog 2013-12-13 Nick Clifton * config/msp430/msp430.c (is_wakeup_func): New function. Returns true if the current function has the wakeup attribute. (msp430_start_function): Note if the function has the wakeup attribute. (msp430_attribute_table): Add wakeup attribute. (msp430_expand_epilogue): Add support for wakeup functions. * config/msp430/msp430.md (disable_interrupts): Emit a NOP after the DINT instruction. * doc/extend.texi: Document the wakeup attribute.
Commit: MSP430: Add wakeup function attribute
[This time with the patch attached] Hi Guys, I have applied the patch below to add support for a wakeup function attribute to the MSP430 backend. The wakeup attribute applies to interrupt functions, and it makes them wake the processor from low power sleep states when the interrupt handler exits. Cheers Nick gcc/ChangeLog 2013-12-13 Nick Clifton * config/msp430/msp430.c (is_wakeup_func): New function. Returns true if the current function has the wakeup attribute. (msp430_start_function): Note if the function has the wakeup attribute. (msp430_attribute_table): Add wakeup attribute. (msp430_expand_epilogue): Add support for wakeup functions. * config/msp430/msp430.md (disable_interrupts): Emit a NOP after the DINT instruction. * doc/extend.texi: Document the wakeup attribute. Index: config/msp430/msp430.c === --- config/msp430/msp430.c (revision 205957) +++ config/msp430/msp430.c (working copy) @@ -966,6 +966,12 @@ return is_attr_func ("interrupt"); } +static bool +is_wakeup_func (void) +{ + return msp430_is_interrupt_func () && is_attr_func ("wakeup"); +} + static inline bool is_naked_func (void) { @@ -1005,6 +1011,8 @@ fprintf (outfile, "reentrant "); if (is_critical_func ()) fprintf (outfile, "critical "); + if (is_wakeup_func ()) + fprintf (outfile, "wakeup "); fprintf (outfile, "\n"); } @@ -1131,6 +1139,7 @@ { "naked", 0, 0, true, false, false, msp430_attr, false }, { "reentrant", 0, 0, true, false, false, msp430_attr, false }, { "critical", 0, 0, true, false, false, msp430_attr, false }, + { "wakeup", 0, 0, true, false, false, msp430_attr, false }, { NULL, 0, 0, false, false, false, NULL,false } }; @@ -1409,6 +1418,14 @@ emit_insn (gen_epilogue_start_marker ()); + if (is_wakeup_func ()) +/* Clear the SCG1, SCG0, OSCOFF and CPUOFF bits in the saved copy of the + status register current residing on the stack. When this function + executes its RETI instruction the SR will be updated with this saved + value, thus ensuring that the processor is woken up from any low power + state in which it may be residing. */ +emit_insn (gen_bic_SR (GEN_INT (0xf0))); + fs = cfun->machine->framesize_locals + cfun->machine->framesize_outgoing; increment_stack (fs); Index: config/msp430/msp430.md === --- config/msp430/msp430.md (revision 205957) +++ config/msp430/msp430.md (working copy) @@ -1253,11 +1253,11 @@ "1" "NOP" ) - + (define_insn "disable_interrupts" [(unspec_volatile [(const_int 0)] UNS_DINT)] "" - "DINT" + "DINT \; NOP" ) (define_insn "enable_interrupts" Index: doc/extend.texi === --- doc/extend.texi (revision 205957) +++ doc/extend.texi (working copy) @@ -2919,6 +2919,13 @@ or @code{critical} attributes. They can have the @code{interrupt} attribute. +@item wakeup +@cindex @code{wakeup} attribute +This attribute only applies to interrupt functions. It is silently +ignored if applied to a non-interrupt function. A wakeup interrupt +function will rouse the processor from any low-power state that it +might be in when the function exits. + @end table On Epiphany targets one or more optional parameters can be added like this:
RFA: Fix test pr32912-2.c for 16-bit targets
Hi Guys, The test gcc/testsuite/gcc.dg/pr32912-2.c fails to execute correctly on targets that use 16-bit integers, because it assumes at least a 32-bit integer. The patch below removes this assumption, and it also tidies up the code slightly so that __SIZEOF_INT__ is only tested in one place, not three. There were no regressions when tested with a i686-pc-linux-gnu or a x86_64-pc-linux-gnu toolchain, and the test was fixed for a rl78-elf toolchain. OK to apply ? Cheers Nick gcc/testsuite/ChangeLog 2013-12-18 Nick Clifton * gcc.dg/pr32912-2.c: Fix for 16-bit targets. Index: gcc/testsuite/gcc.dg/pr32912-2.c === --- gcc/testsuite/gcc.dg/pr32912-2.c(revision 206082) +++ gcc/testsuite/gcc.dg/pr32912-2.c(working copy) @@ -1,14 +1,24 @@ /* { dg-do run } */ /* { dg-options "-O2 -w" } */ -/* { dg-skip-if "TImode not supported" { "avr-*-*" } { "*" } { "" } } */ extern void abort (void); #if(__SIZEOF_INT__ >= 4) -typedef int __m128i __attribute__ ((__vector_size__ (16))); +# define TYPE int +# define TYPED(a) a + +#elif(__SIZEOF_INT__ > 2) +# define TYPE long +# define TYPED(a) a##L + #else -typedef long __m128i __attribute__ ((__vector_size__ (16))); +# define TYPE long long +# define TYPED(a) a##LL #endif + + +typedef TYPE __m128i __attribute__ ((__vector_size__ (16))); + __m128i foo (void) { @@ -26,11 +36,7 @@ int main (void) { -#if(__SIZEOF_INT__ >= 4) - union { __m128i v; int i[sizeof (__m128i) / sizeof (int)]; } u, v; -#else - union { __m128i v; long i[sizeof (__m128i) / sizeof (long)]; } u, v; -#endif + union { __m128i v; TYPE i[sizeof (__m128i) / sizeof (TYPE)]; } u, v; int i; u.v = foo (); @@ -39,9 +45,10 @@ { if (u.i[i] != ~v.i[i]) abort (); + if (i < 3) { - if (u.i[i] != (0x << i)) + if (u.i[i] != (TYPED (0x) << i)) abort (); } else if (u.i[i])
RFA: Use precision in get_mode_bounds()
Hi Guys, I have tracked down a bug reported against the MSP430 (PR target/59613) which turns out to be a generic problem. The function get_mode_bounds() in stor-layout.c computes the minimum and maximum values for a given mode, but it uses the bitsize of the mode not the precision. This is incorrect if the two values are different, (which can happen when the target is using partial integer modes), since values outside of the precision cannot be stored in the mode, no matter how many bits the mode uses. The result, for the MSP430 in LARGE mode, is that calling get_mode_bounds() on PSImode returns a maximum value of -1 instead of 1^20. Note - what happens is that get_mode_bounds computes the maximum as (1 << 31) - 1 and then calls gen_int_mode to create an RTX for this value. But gen_int_mode calls trunc_int_for_mode which uses GET_MODE_PRECISION to determine the sign bits and these bits overwrite some of the zero bits in (1 << 31) - 1 changing it into -1. The result of this behaviour is the bug that I was tracking down. simplify_const_relational_operation() was erroneously discarding a comparison of a pointer against zero, because get_mode_bounds told it that the pointer could only have values between 0 and -1... The proposed patch is simple - use the mode's precision and not its bitsize to compute the bounds. This seems like an obvious fix, but I am not familiar with all of the uses of get_mode_bounds() so maybe there is a situation where using the bitsize is correct. There were no regressions and several fixed test cases with the msp430-elf toolchain that I was using, and no regressions with an i686-pc-linux-gnu toolchain. OK to apply ? Cheers Nick PS. Just found out that the same patch was created by Peter Bigot for a different PR: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52856 gcc/ChangeLog 2013-12-30 Nick Clifton PR target/59631 * stor-layout.c (get_mode_bounds): Use GET_MODE_PRECISION instead of GET_MODE_BITSIZE. Index: stor-layout.c === --- stor-layout.c (revision 206241) +++ stor-layout.c (working copy) @@ -2816,7 +2816,8 @@ enum machine_mode target_mode, rtx *mmin, rtx *mmax) { - unsigned size = GET_MODE_BITSIZE (mode); + /* PR 59613: Use the precision of the mode, not its bitsize. */ + unsigned size = GET_MODE_PRECISION (mode); unsigned HOST_WIDE_INT min_val, max_val; gcc_assert (size <= HOST_BITS_PER_WIDE_INT);
Commit: MSP430: Add %A, %B, %C and %D as selectors for 16-bit parts of a 64-bit operand.
Hi Guys, I am applying the patch below to the MSP430 backend to enable the use of %A, %B, %C and %D in asm statements as selectors of 16-bit parts of a 64-bit value. This is at the request of TI, for compatibility with their compiler. Cheers Nick gcc/ChangeLog 2013-12-30 Nick Clifton * config/msp430/msp430.c (msp430_print_operand): Rename %B to %b and %A to %Q. Add %A, %B, %C and %D as selectors for 16-bit parts of a 64-bit operand. * config/msp430/msp430.md: Replace uses of %B with %b and uses of %A with %q. Index: gcc/config/msp430/msp430.c === --- gcc/config/msp430/msp430.c (revision 206246) +++ gcc/config/msp430/msp430.c (working copy) @@ -1914,6 +1914,24 @@ #undef TARGET_PRINT_OPERAND #define TARGET_PRINT_OPERAND msp430_print_operand +/* A low 16-bits of int/lower of register pair + B high 16-bits of int/higher of register pair + C bits 32-47 of a 64-bit value/reg 3 of a DImode value + D bits 48-63 of a 64-bit value/reg 4 of a DImode value + H like %B (for backwards compatibility) + I inverse of value + L like %A (for backwards compatibility) + O offset of the top of the stack + Q like X but generates an A postfix + R inverse of condition code, unsigned. + X X instruction postfix in large mode + Y value - 4 + Z value - 1 + b .B or .W or .A, depending upon the mode + p bit position + r inverse of condition code + x like X but only for pointers. */ + static void msp430_print_operand (FILE * file, rtx op, int letter) { @@ -1978,7 +1996,7 @@ gcc_assert (CONST_INT_P (op)); fprintf (file, "#%d", 1 << INTVAL (op)); return; -case 'B': +case 'b': switch (GET_MODE (op)) { case QImode: fprintf (file, ".B"); return; @@ -1988,6 +2006,7 @@ default: return; } +case 'A': case 'L': /* Low half. */ switch (GET_CODE (op)) { @@ -2005,6 +2024,7 @@ gcc_unreachable (); } break; +case 'B': case 'H': /* high half */ switch (GET_CODE (op)) { @@ -2023,6 +2043,42 @@ gcc_unreachable (); } break; +case 'C': + switch (GET_CODE (op)) + { + case MEM: + op = adjust_address (op, Pmode, 3); + break; + case REG: + op = gen_rtx_REG (Pmode, REGNO (op) + 2); + break; + case CONST_INT: + op = GEN_INT (INTVAL (op) >> 32); + letter = 0; + break; + default: + /* If you get here, figure out a test case :-) */ + gcc_unreachable (); + } + break; +case 'D': + switch (GET_CODE (op)) + { + case MEM: + op = adjust_address (op, Pmode, 4); + break; + case REG: + op = gen_rtx_REG (Pmode, REGNO (op) + 3); + break; + case CONST_INT: + op = GEN_INT (INTVAL (op) >> 48); + letter = 0; + break; + default: + /* If you get here, figure out a test case :-) */ + gcc_unreachable (); + } + break; case 'X': /* This is used to turn, for example, an ADD opcode into an ADDX @@ -2039,7 +2095,7 @@ fprintf (file, "X"); return; -case 'A': +case 'Q': /* Likewise, for BR -> BRA. */ if (TARGET_LARGE) fprintf (file, "A"); @@ -2053,6 +2109,12 @@ msp430_initial_elimination_offset (ARG_POINTER_REGNUM, STACK_POINTER_REGNUM) - 2); return; + +case 0: + break; +default: + output_operand_lossage ("invalid operand prefix"); + return; } switch (GET_CODE (op)) Index: gcc/config/msp430/msp430.md === --- gcc/config/msp430/msp430.md (revision 206246) +++ gcc/config/msp430/msp430.md (working copy) @@ -87,7 +87,7 @@ [(unspec_volatile [(match_operand 0 "register_operand" "r") (match_operand 1 "immediate_operand" "n")] UNS_PUSHM)] "" - "PUSHM%B0\t%1, %0" + "PUSHM%b0\t%1, %0" ) (define_insn "pop" @@ -105,7 +105,7 @@ ) ;; This is nasty. Operand0 is bogus. It is only there so that we can get a -;; mode for the %B0 to work. We should use operand1 for this, but that does +;; mode for the %b0 to work. We should use operand1 for this, but that does ;; not have a mode. ;; ;; Operand1 is actually a register, but we cannot accept (REG...) because the @@ -115,7 +115,7 @@ ;; because that is the only operator that will omit the # prefix to an ;; integer value. Unfortunately it also inv
Re: RFA: Use precision in get_mode_bounds()
Hi Volker, apparently you added the wrong PR number to the patch. You probably meant PR 59613. Would you mind fixing that in the ChangeLog? Doh! Sorry - fixed. Cheers Nick
RFA: Fix assembler data directives emitted for variable length structures
Hi Guys, Several PRs (28865, 57180 and 59719) have reported that the assembler directives emitted by gcc to describe a structure with a variable length field occupy too much space. That is a serious problem for targets which use section anchors as they rely upon objects in the data area being exactly the size that they are supposed to be. The attached patch fixes the problem by updating the output_constant() function so that it returns the number of bytes that it really did emit, which may be larger than the number of bytes that it was requested to emit. It also adds a couple of tests to the testsuite to check that the desired behaviour is achieved. Tested without regressions on i686-pc-linux-gnu and aarch64-elf toolchains, as well as bootstrapping and regression testing a powerpc64-linux toolchain. OK to apply ? Cheers Nick gcc/ChangeLog 2014-01-13 Nick Clifton PR middle-end/28865 * varasm.c (output_constant): Return the number of bytes actually emitted. (output_constructor_array_range): Update the field size with the number of bytes emitted by output_constant. (output_constructor_regular_field): Likewise. Also do not complain if the total number of bytes emitted is now greater than the expected fieldpos. * output.h (output_constant): Update prototype and descriptive comment. gcc/testsuite/ChangeLog 2014-01-13 Nick Clifton PR middle-end/28865 * gcc.c-torture/compile/pr28865.c: New. * gcc.c-torture/execute/pr28865.c: New. Index: gcc/output.h === --- gcc/output.h (revision 206572) +++ gcc/output.h (working copy) @@ -294,11 +294,13 @@ This includes the pseudo-op such as ".int" or ".byte", and a newline. Assumes output_addressed_constants has been done on EXP already. - Generate exactly SIZE bytes of assembler data, padding at the end - with zeros if necessary. SIZE must always be specified. + Generate at least SIZE bytes of assembler data, padding at the end + with zeros if necessary. SIZE must always be specified. The returned + value is the actual number of bytes of assembler data generated, which + may be bigger than SIZE if the object contains a variable length field. ALIGN is the alignment in bits that may be assumed for the data. */ -extern void output_constant (tree, unsigned HOST_WIDE_INT, unsigned int); +extern unsigned HOST_WIDE_INT output_constant (tree, unsigned HOST_WIDE_INT, unsigned int); /* When outputting delayed branch sequences, this rtx holds the sequence being output. It is null when no delayed branch Index: gcc/varasm.c === --- gcc/varasm.c (revision 206572) +++ gcc/varasm.c (working copy) @@ -4584,8 +4584,10 @@ This includes the pseudo-op such as ".int" or ".byte", and a newline. Assumes output_addressed_constants has been done on EXP already. - Generate exactly SIZE bytes of assembler data, padding at the end - with zeros if necessary. SIZE must always be specified. + Generate at least SIZE bytes of assembler data, padding at the end + with zeros if necessary. SIZE must always be specified. The returned + value is the actual number of bytes of assembler data generated, which + may be bigger than SIZE if the object contains a variable length field. SIZE is important for structure constructors, since trailing members may have been omitted from the constructor. @@ -4600,14 +4602,14 @@ ALIGN is the alignment of the data in bits. */ -void +unsigned HOST_WIDE_INT output_constant (tree exp, unsigned HOST_WIDE_INT size, unsigned int align) { enum tree_code code; unsigned HOST_WIDE_INT thissize; if (size == 0 || flag_syntax_only) -return; +return size; /* See if we're trying to initialize a pointer in a non-default mode to the address of some declaration somewhere. If the target says @@ -4672,7 +4674,7 @@ && vec_safe_is_empty (CONSTRUCTOR_ELTS (exp))) { assemble_zeros (size); - return; + return size; } if (TREE_CODE (exp) == FDESC_EXPR) @@ -4684,7 +4686,7 @@ #else gcc_unreachable (); #endif - return; + return size; } /* Now output the underlying data. If we've handling the padding, return. @@ -4723,8 +4725,7 @@ switch (TREE_CODE (exp)) { case CONSTRUCTOR: - output_constructor (exp, size, align, NULL); - return; + return output_constructor (exp, size, align, NULL); case STRING_CST: thissize = MIN ((unsigned HOST_WIDE_INT)TREE_STRING_LENGTH (exp), size); @@ -4752,11 +4753,10 @@ case RECORD_TYPE: case UNION_TYPE: gcc_assert (TREE_CODE (exp) == CONSTRUCTOR); - output_constructor (exp, size, align, NULL); - return; + retur
s390-linux fails to build
Hi Helmut, Hi Ulrich, Hi Andreas, A toolchain configured as --target=s390-linux currently fails to build gcc because of an undefined function: undefined reference to `s390_host_detect_local_cpu(int, char const**)' Makefile:1858: recipe for target 'xgcc' failed The patch below fixes the problem for me by adding a stub function in s390-common.c, but I am not sure if it is the correct solution. Please can you advise ? Cheers Nick Index: gcc/common/config/s390/s390-common.c === --- gcc/common/config/s390/s390-common.c(revision 226094) +++ gcc/common/config/s390/s390-common.c(working copy) @@ -119,6 +119,14 @@ } } +const char * s390_host_detect_local_cpu (int, const char **) __attribute__((weak)); +const char * +s390_host_detect_local_cpu (int argc ATTRIBUTE_UNUSED, + const char **argv ATTRIBUTE_UNUSED) +{ + return NULL; +} + #undef TARGET_DEFAULT_TARGET_FLAGS #define TARGET_DEFAULT_TARGET_FLAGS (TARGET_DEFAULT)
RFA: RL78: Add an optimization to the addsi3 pattern
Hi DJ, This patch adds a small optimization to the RL78 addsi3 pattern so that it can skip adding in the high part of a constant symbolic address when -mes0 is active. Under these circumstances we know that the address has to be in low memory and that it is invalid to attempt to access anything in high memory relative to this symbol. Tested with no regressions on an rl78-elf toolchain. OK to apply ? Cheers Nick gcc/ChangeLog 2015-07-27 Nick Clifton * config/rl78/rl78.c (rl78_addsi3_internal): New function. Optimizes the case where -mes0 is active and a constant symbolic address is used. * config/rl78/rl78-protos.h: Prototype the new function. * config/rl78/rl78.md (addsi3_internal_real): Call new function. Index: gcc/config/rl78/rl78-protos.h === --- gcc/config/rl78/rl78-protos.h (revision 226240) +++ gcc/config/rl78/rl78-protos.h (working copy) @@ -18,6 +18,7 @@ along with GCC; see the file COPYING3. If not see <http://www.gnu.org/licenses/>. */ +const char *rl78_addsi3_internal (rtx *, unsigned int); void rl78_emit_eh_epilogue (rtx); void rl78_expand_compare (rtx *); void rl78_expand_movsi (rtx *); Index: gcc/config/rl78/rl78.c === --- gcc/config/rl78/rl78.c (revision 226240) +++ gcc/config/rl78/rl78.c (working copy) @@ -4638,6 +4641,34 @@ return res; } + +const char * +rl78_addsi3_internal (rtx * operands, unsigned int alternative) +{ + /* Issue 5905133: + If we are adding in a constant symbolic address when -mes0 + is active then we know that the address must be <64K and + that it is invalid to access anything above 64K relative to + this address. So we can skip adding in the high bytes. */ + if (TARGET_ES0 + && GET_CODE (operands[2]) == SYMBOL_REF + && TREE_CODE (SYMBOL_REF_DECL (operands[2])) == VAR_DECL + && TREE_READONLY (SYMBOL_REF_DECL (operands[2])) + && ! TREE_SIDE_EFFECTS (SYMBOL_REF_DECL (operands[2]))) +return "movw ax, %h1\n\taddw ax, %h2\n\tmovw %h0, ax"; + + switch (alternative) +{ +case 0: +case 1: + return "movw ax, %h1\n\taddw ax, %h2\n\tmovw %h0, ax\n\tmovw ax, %H1\n\tsknc\n\tincw ax\n\taddw ax, %H2\n\tmovw %H0, ax"; +case 2: + return "movw ax, %h1\n\taddw ax,%h2\n\tmovw bc, ax\n\tmovw ax, %H1\n\tsknc\n\tincw ax\n\taddw ax, %H2\n\tmovw %H0, ax\n\tmovw ax, bc\n\tmovw %h0, ax"; +default: + gcc_unreachable (); +} +} + #undef TARGET_PREFERRED_RELOAD_CLASS #define TARGET_PREFERRED_RELOAD_CLASS rl78_preferred_reload_class Index: gcc/config/rl78/rl78.md === --- gcc/config/rl78/rl78.md (revision 226240) +++ gcc/config/rl78/rl78.md (working copy) @@ -243,10 +243,7 @@ (clobber (reg:HI BC_REG)) ] "rl78_real_insns_ok ()" - "@ - movw ax,%h1 \;addw ax,%h2 \;movw %h0, ax \;movw ax,%H1 \;sknc \;incw ax \;addw ax,%H2 \;movw %H0,ax - movw ax,%h1 \;addw ax,%h2 \;movw %h0, ax \;movw ax,%H1 \;sknc \;incw ax \;addw ax,%H2 \;movw %H0,ax - movw ax,%h1 \;addw ax,%h2 \;movw bc, ax \;movw ax,%H1 \;sknc \;incw ax \;addw ax,%H2 \;movw %H0,ax \;movw ax,bc \;movw %h0, ax" + { return rl78_addsi3_internal (operands, which_alternative); } [(set_attr "valloc" "macax")] )
Commit: M32R: Fix handling of the __model__ attribute
Hi Guys, I am checking in the patch below to fix the M32R port's handling of its __model__ attribute. This attribute takes a parameter identifying the memory model to use, but gcc was not being told to expect this identifier. Cheers Nick gcc/ChangeLog 2015-07-31 Nick Clifton * config/m32r/m32r.c (m32r_attribute_identifier): New function. Returns true for __model__. (TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P): Define. Index: gcc/config/m32r/m32r.c === --- gcc/config/m32r/m32r.c (revision 226444) +++ gcc/config/m32r/m32r.c (working copy) @@ -113,6 +113,7 @@ static void m32r_conditional_register_usage (void); static void m32r_trampoline_init (rtx, tree, rtx); static bool m32r_legitimate_constant_p (machine_mode, rtx); +static bool m32r_attribute_identifier (const_tree); /* M32R specific attributes. */ @@ -129,6 +130,8 @@ /* Initialize the GCC target structure. */ #undef TARGET_ATTRIBUTE_TABLE #define TARGET_ATTRIBUTE_TABLE m32r_attribute_table +#undef TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P +#define TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P m32r_attribute_identifier #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P m32r_legitimate_address_p @@ -404,6 +407,13 @@ return NULL_TREE; } + +static bool +m32r_attribute_identifier (const_tree name) +{ + return strcmp (IDENTIFIER_POINTER (name), "model") == 0 +|| strcmp (IDENTIFIER_POINTER (name), "__model__") == 0; +} /* Encode section information of DECL, which is either a VAR_DECL, FUNCTION_DECL, STRING_CST, CONSTRUCTOR, or ???.
RFA: RL78: Remove far operand optimization in rl78_force_nonfar_3
Hi DJ, It turns out that the optimization in rl78_force_nonfar_3 to allow some special cases to be kept in far pointers does not always work. The test case included with this patch will trigger ICEs if the optimization is allowed to persist. So, may I check this patch in please ? Cheers Nick gcc/ChangeLog 2015-08-04 Nick Clifton * config/rl78/rl78.c (rl78_force_nonfar_3): Remove optimization to allow identical far pointers to remain. gcc/testsuite/ChangeLog 2015-08-04 Nick Clifton * gcc.target/rl78: New directory. * gcc.target/rl78/rl78.exp: New file: Test driver. * gcc.target/rl78/test_addm3.c: New file: Test adds. Index: gcc/config/rl78/rl78.c === --- gcc/config/rl78/rl78.c (revision 226548) +++ gcc/config/rl78/rl78.c (working copy) @@ -608,13 +608,6 @@ int did = 0; rtx temp_reg = NULL; - /* As an exception, we allow two far operands if they're identical - and the third operand is not a MEM. This allows global variables - to be incremented, for example. */ - if (rtx_equal_p (operands[0], operands[1]) - && ! MEM_P (operands[2])) -return 0; - /* FIXME: Likewise. */ if (rl78_far_p (operands[1])) { --- /dev/null 2015-08-04 08:05:06.160754276 +0100 +++ gcc/testsuite/gcc.target/rl78/rl78.exp 2015-08-04 14:03:20.759389085 +0100 @@ -0,0 +1,43 @@ +# Copyright (C) 2015 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +# GCC testsuite that uses the `dg.exp' driver. + +# Exit immediately if this isn't the right target. +if { ![istarget rl78-*-*] } then { + return +} + +# Load support procs. +load_lib gcc-dg.exp + +# If a testcase doesn't have special options, use these. +global DEFAULT_CFLAGS +if ![info exists DEFAULT_CFLAGS] then { +set DEFAULT_CFLAGS "" +} + +# Initialize `dg'. +dg-init + +# Find all tests +set tests [lsort [find $srcdir/$subdir *.\[cS\]]] + +# Main loop. +gcc-dg-runtest $tests "" $DEFAULT_CFLAGS + +# All done. +dg-finish --- /dev/null 2015-08-04 08:05:06.160754276 +0100 +++ gcc/testsuite/gcc.target/rl78/test_addm3.c 2015-08-04 15:12:53.509456677 +0100 @@ -0,0 +1,100 @@ +/* Remove `-ansi' from options to enable the use of __far and long long. */ +/* { dg-options "" } */ + +#define ADD(TYPE, name) \ + TYPE \ + add##name(TYPE a, TYPE b) \ + { \ +return a + b;\ + } \ + +#define ADDIMM(TYPE, name) \ + TYPE \ + addimm##name(TYPE a)\ + { \ +return a + 50;\ + } \ + +#define ADDFAR(TYPE, name) \ + TYPE __far gf##name;\ + void \ + addfar##name(TYPE __far *pa, TYPE b) \ + { \ +gf##name += b;\ +*pa += 50; \ + } \ + + +ADD (char, qi3) +ADD (int, hi3) +ADD (long, si3) +ADD (long long, di3) +ADD (float, sf3) +ADD (double, df3) + +ADDIMM (char, qi3) +ADDIMM (int, hi3) +ADDIMM (long, si3) +ADDIMM (long long, di3) +ADDIMM (float, sf3) +ADDIMM (double, df3) + +ADDFAR (char, qi3) +ADDFAR (int, hi3) +ADDFAR (long, si3) +ADDFAR (long long, di3) +ADDFAR (float, sf3) +ADDFAR (double, df3) + +char aqi1, aqi2; +int ahi1, ahi2; +long asi1, asi2; +long long adi1, adi2; +float af1, af2; +double ad1, ad2; + +void +testglobal (void) +{ + aqi1 += aqi2; + ahi1 += ahi2; + asi1 += asi2; + adi1 += adi2; + af1 += af2; + ad1 += ad2; +} + +void +testglobal2 (void) +{ + aqi1 += 10; + ahi1 += 11; + asi1 += 12; + adi1 += 13; + af1 += 2.0; + ad1 += 4.0; +} + +void +testptr (char *aqi1, int *ahi1, long *asi1, long long *adi1, float *af1, double *ad1, + char *aqi2, int *ahi2, long *asi2, long long *adi2, float *af2, double *ad2) +{ + *aqi1 += *aqi2; + *ahi1 += *ahi2; + *asi1 += *asi2; + *adi1 += *adi2; + *af1 += *af2; + *ad1 += *ad2; +} + +void +testptr2 (char *aqi1, int *ahi1, long *asi1, long long *adi1, float *af1, double *ad1) +{ + *aqi1 += 5; + *ahi1 += 10; + *asi1 += 11; + *adi1 += 12; + *af1 += 4.5; + *ad1 += 5.5; +} +
RFA: RL78: Fix multiply costs when optimizing for size
Hi DJ, The patch below fixes a small problem with the RL78 backend. When optimizing for size it is better to use a slow multiply instruction than a faster, but larger, shift sequence. So the patch tweaks the rtx costs for MULT insns when speed is not a priority. Tested with no regressions on an rl78-elf toolchain. OK to apply ? Cheers Nick gcc/ChangeLog 2015-08-05 Nick Clifton * config/rl78/rl78.c (rl78_rtx_costs): Treat MULT insns as cheap if optimizing for size. Index: gcc/config/rl78/rl78.c === RCS file: /cvs/cvsfiles/gnupro/gcc/config/rl78/rl78.c,v retrieving revision 1.12.6.15 diff -u -3 -p -r1.12.6.15 rl78.c --- gcc/config/rl78/rl78.c 29 Jul 2015 12:24:04 - 1.12.6.15 +++ gcc/config/rl78/rl78.c 30 Jul 2015 15:20:10 - @@ -4161,7 +4161,9 @@ static bool rl78_rtx_costs (rtx x, switch (code) { case MULT: - if (RL78_MUL_G14) + if (! speed) + * total = COSTS_N_INSNS (5); + else if (RL78_MUL_G14) *total = COSTS_N_INSNS (14); else if (RL78_MUL_G13) *total = COSTS_N_INSNS (29);
Re: RFA: RL78: Remove far operand optimization in rl78_force_nonfar_3
Hi DJ, This is OK, but note that it prevents some operations like: __far int i; foo() { i ++; } from being implemented with a minimum set of opcodes. This might be particularly troublesome for volatile far things. Right - it is something I will have to look into. Cheers Nick
RFA: LM32: Configure with newlib-stdint.h
Hi Sebastien, I recently found that I could not build libstdc++ for the lm32-elf target because the port lacked a definition of __INTPTR_TYPE__. I tracked this down to the fact that INTPTR_TYPE was not being defined when building gcc, and this in turn was due to the fact that the newlib stdint types were not being defined. So please may I apply the patch below to correct this problem ? Cheers Nick gcc/ChangeLog 2015-10-06 Nick Clifton * config.gcc (lm32-elf): Add newlib-stdint.h to tm_file. Index: gcc/config.gcc === --- gcc/config.gcc (revision 228513) +++ gcc/config.gcc (working copy) @@ -1834,7 +1834,7 @@ md_file=iq2000/iq2000.md ;; lm32-*-elf*) -tm_file="dbxelf.h elfos.h ${tm_file}" +tm_file="dbxelf.h elfos.h ${tm_file} newlib-stdint.h" tmake_file="${tmake_file} lm32/t-lm32" ;; lm32-*-rtems*)
Commit: RL78: Improve multiplication cost estimate
Hi DJ, This is the patch that I have checked in. Cheers Nick gcc/ChangeLog 2015-10-06 Nick Clifton * config/rl78/rl78.c (rl78_rtx_costs): Improve cost estimates for multiplication. Index: gcc/config/rl78/rl78.c === --- gcc/config/rl78/rl78.c (revision 228513) +++ gcc/config/rl78/rl78.c (working copy) @@ -4165,12 +4165,31 @@ return true; } + if (mode == HImode) +{ + if (code == MULT && ! speed) + { + * total = COSTS_N_INSNS (8); + return true; + } + return false; +} + if (mode == SImode) { switch (code) { case MULT: - if (RL78_MUL_G14) + if (! speed) + /* If we are compiling for space then we do not want to use the + inline SImode multiplication patterns or shift sequences. + The cost is not set to 1 or 5 however as we have to allow for + the possibility that we might be converting a leaf function + into a non-leaf function. (There is no way to tell here). + A value of 13 seems to be a reasonable compromise for the + moment. */ + * total = COSTS_N_INSNS (13); + else if (RL78_MUL_G14) *total = COSTS_N_INSNS (14); else if (RL78_MUL_G13) *total = COSTS_N_INSNS (29);
Commit: MSP430: Add support for persistent data
Hi Guys, I am checking in the attached patch to add support for persistent data to the MSP430 port. Persistent data retains its current value across processor resets (because it is held in flash), so its value must be set once - when the program is loaded - and then never changed by the runtime support code again. The patch also includes some documentation for this new feature and a new testcase to make sure that it works. An associated libgloss patch is needed in order to update the linker scripts involved, and I will be applying that patch shortly. Tested with no regressions on an msp430-elf toolchain. Cheers Nick gcc/ChangeLog 2015-10-06 Nick Clifton * config/msp430/msp430.c (ATTR_NOINIT): New constant. (ATTR_PERSIST): New constant. (msp430_data_attr): New function - verifies an attribute that only applies to variables. (msp430_attributes): Add noinit and persistent attributes. (noinit_section): New variable. (presis_section): New variable. (TARGET_ASM_INIT_SECTIONS): Define. (msp430_init_sections): New function - initialises the noinit and persist section variables. (msp430_select_section): Add support for noinit and persist attributes. (msp430_section_type_flags): Likewise. * doc/extend.texi: Document the reent, critical, wakeup, noinit and persistent attributes. gcc/testsuite/ChangeLog 2015-10-06 Nick Clifton * gcc.target/msp430: New directory. * gcc.target/msp430/msp430.exp: New file. Runs MSP430 specific tests. * gcc.target/msp430/data-attributes.c: New file. Checks the noinit and persistent data attributes. msp430.persistent.patch.2 Description: Unix manual page
RFA/RFC: insns that do not start a source line
Hi Guys, Sometimes gcc can generate instructions out of order with respect to lines of source code, and this can lead to problems for debuggers. For example, consider this source code snippet: v = 0; /* Line 31 */ goto b;/* Line 32 */ a: v++; /* Line 33 */ b: if (v < 3) goto a; /* Line 34 */ Compiled without optimization, but with -g, gcc will produce code like this: 400692: c7 05 94 09 20 00 00movl $0x0,0x200994(%rip) 400699: 00 00 00 40069c: eb 10 jmp4006ae 40069e: 90 nop 40069f: 8b 05 8b 09 20 00 mov0x20098b(%rip),%eax 4006a5: 83 c0 01add$0x1,%eax 4006a8: 89 05 82 09 20 00 mov%eax,0x200982(%rip) 4006ae: 8b 05 7c 09 20 00 mov0x20097c(%rip),%eax 4006b4: 83 f8 02cmp$0x2,%eax 4006b7: 7e e5 jle40069e 4006b9: 90 nop This example uses x86_64, but the same problem occurs for most architectures. Note the two NOP instructions. The DWARF line table information produced for this code looks like this: advance Address by 5 to 0x400692 and Line by 7 to 31 advance Address by 10 to 0x40069c and Line by 1 to 32 advance Address by 2 to 0x40069e and Line by 2 to 34 advance Address by 1 to 0x40069f and Line by -1 to 33 advance Address by 15 to 0x4006ae and Line by 1 to 34 advance Address by 11 to 0x4006b9 and Line by 1 to 35 (Note the backwards movement of the line number to the start of line 33). The problem is line 34. According to the line table it starts at address 0x40069e whereas in reality it starts at 0x4006ae. What has happened is that gcc has generated a NOP instruction to be the destination of the goto on line 34 - ie the label a: - but rather than associate it with line 33, it has associated it with line 34. This means that line 34 now appears twice in the line number table. I am offering the patch below as a fix for this problem, although I am open to suggestions for better solutions. The patch creates a hash table for instructions which gcc knows should not be considered to be the start of a source code line, then it checks this table when noticing source line changes. The patch only adds the NOP instructions generated for the test code above, although I expect that in the future there may be other places that need to record this information. Tested with no regressions on an x86_64-pc-linux toolchain. Comments / questions / approval ? Cheers Nick gcc/ChangeLog 2015-10-22 Nick Clifton * cfgrtl.c (not_a_stmt_hasher): New struct. (not_a_stmt_htab): New hash table. (is_a_stmt): New function. Returns true iff the given insn is on the list of insns known not to start a line of source code. (fixup_reorder_chain): Add generated NOP instructions to the list of instructions that are known not to start a source line. * cfgrtl.h (is_a_stmt): Add prototype. * final.c (notice_source_line): Use the new function to set the is_stmt return value. Index: gcc/cfgrtl.c === --- gcc/cfgrtl.c(revision 229162) +++ gcc/cfgrtl.c(working copy) @@ -3665,7 +3665,21 @@ compact_blocks (); } +struct not_a_stmt_hasher : ggc_cache_ptr_hash +{ + static hashval_t hash (rtx x) { return htab_hash_pointer (x); } + static bool equal (rtx a, rtx b) { return a == b; } +}; +static GTY((cache)) hash_table *not_a_stmt_htab; + +/* Return TRUE if INSN is on the list of not-an-insn. */ +bool +is_a_stmt (rtx_insn *insn) +{ + return not_a_stmt_htab == NULL || ! not_a_stmt_htab->find (insn); +} + /* Given a reorder chain, rearrange the code to match. */ static void @@ -3944,8 +3958,18 @@ } nb = split_edge (e); if (!INSN_P (BB_END (nb))) - BB_END (nb) = emit_insn_after_noloc (gen_nop (), BB_END (nb), -nb); + { + BB_END (nb) = emit_insn_after_noloc (gen_nop (), BB_END (nb), + nb); + + /* Note that this insn is a fake and should not be considered +to be the starting insn of a line of source code. */ + if (not_a_stmt_htab == NULL) + not_a_stmt_htab = hash_table::create_ggc (17); + rtx_def ** slot; + slot = not_a_stmt_htab->find_slot (BB_END (nb), INSERT); + * slot = BB_END (nb); + } INSN_LOCATION (BB_END (nb)) = e->goto_locus; /* If there are other incoming edges to the destination bloc
Re: RFA/RFC: insns that do not start a source line
Hi Bernd, Could you point me at the code generating these NOPs It is in cfgrtl.c:fixup_reorder_chain() nb = split_edge (e); if (!INSN_P (BB_END (nb))) BB_END (nb) = emit_insn_after_noloc (gen_nop (), BB_END (nb), nb); INSN_LOCATION (BB_END (nb)) = e->goto_locus; and maybe a testcase)? Full testcase attached. Compile with -O0 -g. Also, is this the only case you know of or are there others? Sorry, I have not investigated this. The problem was reported to me by a GDB engineer, who was investigating gdb's failure to apply breakpoints to the correct locations under some circumstances (such as the code in the testcase). There may well be other tests that have similar properties, especially loops that are unrolled or goto statements that cross basic blocks. Cheers Nick volatile int v; volatile int w; void loop_test (void) { v = 0; while (v < 3) /* Loop 1 condition */ { v++; /* Loop 1 increment */ } v = -42; for (v = 0; v < 3; ) /* Loop 2 */ { v++; /* Loop 2 increment */ } v = -42; w = 42; for (v = 0; /* Loop 3 initialization */ v < 3; /* Loop 3 condition */ v++) /* Loop 3 increment */ { w = w - v; } v = 0; goto b; /* Loop 4 initial goto */ a: v++; b: if (v < 3) goto a; /* Loop 4 condition */ } int main (int argc, char **argv) { loop_test (); return 0; }
Commit: MSP430: Pass silicon errata options on to the assembler
Hi Guys, I am checking in the patch below to allow gcc to pass the new MSP430 -msilicon-errata and -msilicon-errata-warn option on to the assembler. Cheers Nick gcc/ChangeLog 2015-10-22 Nick Clifton * config/msp430/msp430.opt: Add -msilicon-errata and -msilicon-errata-warn. * config/msp430/msp430.h (ASM_SPEC): Pass new options on to assembler. * doc/invoke.texi: Document new options. Index: gcc/config/msp430/msp430.h === --- gcc/config/msp430/msp430.h (revision 229176) +++ gcc/config/msp430/msp430.h (working copy) @@ -56,6 +56,8 @@ "%{mrelax=-mQ} " /* Pass the relax option on to the assembler. */ \ "%{mlarge:-ml} " /* Tell the assembler if we are building for the LARGE pointer model. */ \ "%{!msim:-md} %{msim:%{mlarge:-md}} " /* Copy data from ROM to RAM if necessary. */ \ + "%{msilicon-errata=*:-msilicon-errata=%*} " /* Pass on -msilicon-errata. */ \ + "%{msilicon-errata-warn=*:-msilicon-errata-warn=%*} " /* Pass on -msilicon-errata-warn. */ \ "%{ffunction-sections:-gdwarf-sections} " /* If function sections are being created then create DWARF line number sections as well. */ /* Enable linker section garbage collection by default, unless we Index: gcc/config/msp430/msp430.opt === --- gcc/config/msp430/msp430.opt(revision 229176) +++ gcc/config/msp430/msp430.opt(working copy) @@ -80,3 +80,11 @@ EnumValue Enum(msp430_regions) String(upper) Value(UPPER) + +msilicon-errata= +Target Joined RejectNegative Report ToLower +Passes on a request to the assembler to enable fixes for various silicon errata + +msilicon-errata-warn= +Target Joined RejectNegative Report ToLower +Passes on a request to the assembler to warn about various silicon errata Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 229176) +++ gcc/doc/invoke.texi (working copy) @@ -841,6 +841,7 @@ @emph{MSP430 Options} @gccoptlist{-msim -masm-hex -mmcu= -mcpu= -mlarge -msmall -mrelax @gol -mcode-region= -mdata-region= @gol +-msilicon-errata= -msilicon-errata-warn= @gol -mhwmult= -minrt} @emph{NDS32 Options} @@ -18449,6 +18450,16 @@ linker script and how it assigns the standard sections (.text, .data etc) to the memory regions. +@item -msilicon-errata= +@opindex msilicon-errata +This option passes on a request to assembler to enable the fixes for +the names silicon errata. + +@item -msilicon-errata-warn= +@opindex msilicon-errata-warn +This option passes on a request to the assembler to enable warning +messages when a silicon errata might need to be applied. + @end table @node NDS32 Options
Commit: Fix spelling mistake in RL78 documentation
Hi Guys, I am checking in the patch below to fix a small spelling mistake that I recently introduced to the RL78 documentation: gcc/ChangeLog 2015-10-26 Nick Clifton * doc/invoke.texi (RL78 Options): Fix spelling mistake. Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 229376) +++ gcc/doc/invoke.texi (working copy) @@ -19118,7 +19118,7 @@ @option{-mmul=none} option on the command line. Thus specifying @option{-mcpu=g13} enables the use of the G13 hardware multiply peripheral and specifying @option{-mcpu=g10} disables the use of -hardware multipications altogether. +hardware multiplications altogether. Note, although the RL78/G14 core is the default target, specifying @option{-mcpu=g14} or @option{-mcpu=rl78} on the command line does
RFC/RFA: Fix bug with REE optimization corrupting extended registers
Hi Guys, I recently discovered a bug in the current Redundant Extension Elimination optimization. If the candidate extension instruction increases the number of hard registers used, the pass does not check to see if these extra registers are live between the definition and the extension. For example: (insn 44 (set (reg:QI r11) (mem:QI (reg:HI r20))) (insn 45 (set (reg:QI r10) (mem:QI (reg:HI r18))) [...] (insn 71 (set (reg:HI r14) (zero_extend:HI (reg:QI r11))) [...] (insn 88 (set (reg:HI r10) (zero_extend:HI (reg:QI r10))) (This is on the RL78 target where HImode values occupy two hard registers and QImode values only one. The bug however is generic, not RL78 specific). The REE pass transforms this into: (insn 44 (set (reg:QI r11) (mem:QI (reg:HI r20))) (insn 45 (set (reg:HI r10) (zero_extend:HI (mem:QI (reg:HI r18 [...] (insn 71 (set (reg:HI r14) (zero_extend:HI (reg:QI r11))) [...] (insn 88 deleted) Note how the new set at insn 45 clobbers the value loaded by insn 44 into r11. The patch below is my attempt to fix this. It is not correct however. I could not work out how to determine if a given hard register is live at any point between two insns. So instead I used the liveness information associated with the basic block containing the definition. If one of the extended registers is live or becomes live in this block, and this block is not the same block as the one containing the extension insn, then I stop the optimization. This works for the RL78 for all of the cases that I could find. So ... comments please ? Will gcc ever generate a single basic block that contains both the definition and the extension instructions, but also where the extra hard registers are used for another purpose as well ? Tested with no regressions (or fixes) on an x86-pc-linux-gnu target. Also tested with no regression and 7 fixes on an rl78-elf target. Cheers Nick gcc/ChangeLog 2015-11-18 Nick Clifton * ree.c (regs_live_between): New function. (add_removable_extension): If the extension uses more hard registers than the definition then check that the extra hard registers are not live between the definition and the extension. Index: gcc/ree.c === --- gcc/ree.c (revision 230517) +++ gcc/ree.c (working copy) @@ -952,6 +952,49 @@ return false; } +/* Returns TRUE if any of the registers [start, stop) are live between DEF and up + to, but not including, INSN. */ + +static bool +regs_live_between (rtx_insn * insn, + struct df_link * def, + unsigned int start, + unsigned int stop) +{ + basic_block bb; + + /* FIXME: This is an incomplete test. It only checks the DEF and USE states of the + registers in basic block of DEF. If any of them match the start-stop range and + the INSN is not in the same block, then it is assumed that the registers must be + live. */ + /* These first few checks are just paranoia... */ + if (def != NULL + && def->ref != NULL + && def->ref->base.insn_info != NULL + && def->ref->base.insn_info->insn != NULL + && (bb = BLOCK_FOR_INSN (def->ref->base.insn_info->insn)) != NULL + && bb != BLOCK_FOR_INSN (insn)) +{ + struct df_lr_bb_info * bb_info = df_lr_get_bb_info (bb->index); + bitmap_iterator bi; + unsigned int reg; + + EXECUTE_IF_SET_IN_BITMAP (& bb_info->def, 0, reg, bi) + { + if (reg >= start && reg < stop) + return true; + } + + EXECUTE_IF_SET_IN_BITMAP (& bb_info->out, 0, reg, bi) + { + if (reg >= start && reg < stop) + return true; + } +} + + return false; +} + /* Add an extension pattern that could be eliminated. */ static void @@ -1080,6 +1123,30 @@ } } + /* Fourth, if the extended version occupies more registers than +the original version then make sure that these extra registers +are not live between the definition and the extension. */ + if (HARD_REGNO_NREGS (REGNO (dest), mode) + > HARD_REGNO_NREGS (REGNO (reg), GET_MODE (reg))) + { + unsigned int start_reg = REGNO (reg) + HARD_REGNO_NREGS (REGNO (reg), GET_MODE (reg)); + unsigned int stop_reg = REGNO (reg) + HARD_REGNO_NREGS (REGNO (dest), mode); + + for (def = defs; def; def = def->next) + { + if (regs_live_between (insn, def, start_reg, stop_reg)) + { + if (dump_file) + { + fprintf (dump_file, "Cannot eliminate extension:\n"); +
Re: RFC/RFA: Fix bug with REE optimization corrupting extended registers
Hi Bernd, I had a look around. There's code testing HARD_REGNO_NREGS in ree.c:combine_set_extension. It's inside #if 0, and labelled "temporarily disabled". See if enabling that helps you? (Jeff, that #if 0 was added by you). I suspect that the code was disabled because it prevented too many useful optimization opportunities. The code there would solve this problem, but the approach is is overly cautious, since it disables the optimization for all extensions that increase the number of hard registers used. Some of these will be viable candidates, provided that the extra hard registers are no used. (This is certainly true for the RL78, where the (patched) optimization does improve code, even though the widening does use extra registers). Cheers Nick
Re: RFC/RFA: Fix bug with REE optimization corrupting extended registers
Hi Jeff, The code there would solve this problem, but the approach is is overly cautious, since it disables the optimization for all extensions that increase the number of hard registers used. Some of these will be viable candidates, provided that the extra hard registers are no used. (This is certainly true for the RL78, where the (patched) optimization does improve code, even though the widening does use extra registers). Nick -- can you pass along your testcode? Sure - this is for the RL78 toolchain. In theory the problem is generic, but I have not tested other toolchains. Compile the gcc.c-torture/execute/pr42833.c or gcc.c-torture/execute/strct-stdarg-1.c tests at -O1 or higher with -free also specified on the command line. Without -free these tests pass. With -free they fail. Cheers Nick
Commit: MSP430: Reduce number of multilibs
Hi Guys, I am applying the attached patch to reduce the number of multilibs for the MSP430 target. This is at the request of TI, on behalf of their customers, who complained that the toolchain was too large. The patch only affects MSP430 specific files, and parts of files. It does not touch anything else. The patch removes the multilibs based upon the version of hardware multiply support that is used, and instead creates a separate set of libraries just containing the multiply routines. All normal libraries, and user created object files, now call the software multiply routines by default[*]. At link time however one of the specific hardware multiply libraries can be linked in to provide hardware based alternatives to the software multiply functions. As a side effect the patch also fixes a couple of unexpected failures in the gcc testsuite (gcc.dg/cleanup-[12|13|5].c). It also adds some new tests the MSP430 specific section of the gcc testsuite that check the behaviour of the multiply functions for all possible variations oh hardware multiply support. Cheers Nick [*] There is an exception to the all-files-call-software-multiply rule. If a file is compiled at -O3 or above, with a hardware multiply type specified as well, then the hardware functions will be used inline when appropriate. Hence no library should ever be compiled in this way, unless the builder is sure that it will only ever be used on the appropriate hardware. gcc/ChangeLog 2015-11-25 Nick Clifton * config.gcc (extra_gcc_objs): Define for MSP430. * common/config/msp430/msp430-common.c (msp430_handle_option): Pass both -mmcu and -mcpu on to the back end if they are both defined. * config/msp430/msp430.c (hwmult_name): New function. (msp430_option_override): If an unrecognised MCU name is detected only warn if the user has not provided suitable -mhwmult and -mcpu options. Use msp430_warn_mcu to control warning messages. Generate warnings about conflicts between -mmcu and -mcpu and -mhwmult options. If neither -mcpu nor -mmcu have been specified but -mhwmult= f5series has the select the 430X isa. (msp430_no_hwmult): If -mmcu has not been specified and msp430_hwmult_type is AUTO then return true. * config/msp430/msp430.h (EXTRA_SPEC_FUNCTIONS): Define. (LIB_SPEC): Add hardware multiply library selection. * config/msp430/t-msp430: Delete hardware multiply multilibs. Add rule to build driver-msp430.o * config/msp430/driver-msp430.c: New file. * config/msp430/msp430.opt (warn-mcu): New option. * doc/invoke.texi: Update description of -mhwmult=auto. Document -mwarn-mcu option. gcc/testsuite/ChangeLog 2015-11-25 Nick Clifton * gcc.target/msp430/msp_abi_div_funcs.c: New test. * gcc.target/msp430/mul_main.h: New test support file. * gcc.target/msp430/mul_none.c: New test. * gcc.target/msp430/mul_16bit.c: New test. * gcc.target/msp430/mul_32bit.c: New test. * gcc.target/msp430/mul_f5.c: New test. libgcc/ChangeLog 2015-12-04 Nick Clifton * config/msp430/mpy.c (__mulhi3): Use a faster algorithm. Allow for the second argument being negative. * config.host (extra_parts): Define for MSP430. Create separate libraries for each of the hardware multiply formats. * config/msp430/lib2hw_mul.S: Build only the multiply routines that are needed. * config/msp430/lib2mul.c: Likewise. * config/msp430/t-msp430 (LIB2ADD): Remove lib2hw_mul.S. Add rules to build hardware multiply libraries. * config/msp430/lib2divSI.c: (__mspabi_divlu): Alias for __mspabi_divul function. (__mspabi_divllu): New stub function. msp430.hwmul.patch.2.xz Description: application/xz
RFA: Enhance information recorded by -frecord-gcc-switches
Hi Guys, The -frecord-gcc-switches option records the gcc command line. It does not however expand options like -O2 into the optimizations that this enables. Thus if a user wants to know if a specific optimization was used when creating an object file, (or library or executable), they will have to reverse engineer the compilation process. Which may or may not be possible. The attached patch is a proposal to address this problem by making -frecord-gcc-switches also record all the enabled options. This does make object files bigger, but this cannot be helped. The enhancement is not enabled by default however, instead a second command line option must be used. In a possibly contentious move I chose to reuse the -fverbose-asm option, rather than creating a new one. I did this because a) it simplifies the patch, b) we have more than enough switch recording options already, c) it does not conflict with the current use of -fverbose-asm and d) it ties in nicely with the name of the option. Tested, with no regressions on an x86_64-pc-linux-gnu target, and built for a variety of other targets. OK to apply ? Cheers Nick gcc/ChangeLog 2017-06-08 Nick Clifton * varasm.c (dump_enabled_to_asm_out_file): New function. Prints enabled options to the asm_out_file. (elf_record_gcc_switches): If verbose-asm is set then also dump all enabled options to the asm file. * toplec.c (print_switch_values): Convert from static to global. * doc/invoke.texi (-fverbose-asm): Mention its effect on the -frecord-gcc-switches option. (-frecord-gcc-switches): Refactor the description and add details of how -fverbose-asm modifies its behaviour. Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 249007) +++ gcc/doc/invoke.texi (working copy) @@ -12281,10 +12281,13 @@ who actually need to read the generated assembly code (perhaps while debugging the compiler itself). -@option{-fno-verbose-asm}, the default, causes the -extra information to be omitted and is useful when comparing two assembler -files. +This switch can also be used to modify the behaviour of the +@option{-frecord-gcc-switches} switch, making it record extra +information in the object file. +@option{-fno-verbose-asm}, the default, causes the extra information +to be omitted and is useful when comparing two assembler files. + The added comments include: @itemize @bullet @@ -12370,17 +12373,26 @@ @item -frecord-gcc-switches @opindex frecord-gcc-switches -This switch causes the command line used to invoke the -compiler to be recorded into the object file that is being created. -This switch is only implemented on some targets and the exact format -of the recording is target and binary file format dependent, but it -usually takes the form of a section containing ASCII text. This -switch is related to the @option{-fverbose-asm} switch, but that -switch only records information in the assembler output file as -comments, so it never reaches the object file. -See also @option{-grecord-gcc-switches} for another -way of storing compiler options into the object file. +This switch causes the command line used to invoke the compiler to be +recorded into the object file that is being created. This switch is +only implemented on some targets and the exact format of the recording +is target and binary file format dependent, but it usually takes the +form of a section containing ASCII text. +The related switch @option{-fverbose-asm} switch performs a similar +task but it only records the information in the assembler output file +as comments, so it never reaches the object file. + +If both @option{-fverbose-asm} and @option{-frecord-gcc-switches} are +enabled together then @option{-frecord-gcc-switches} will record all +enabled switches, not just those specified on the command line. Thus +if the command line includes @option{-O2} then all optimizations +enabled by that switch will be recorded in the object file, along with +the presence of @option{-O2} itself. + +See also @option{-grecord-gcc-switches} for another way of storing +compiler options into an object file. + @item -fpic @opindex fpic @cindex global offset table Index: gcc/toplev.c === --- gcc/toplev.c (revision 249007) +++ gcc/toplev.c (working copy) @@ -809,7 +809,9 @@ Each line begins with INDENT and ends with TERM. Each switch is separated from the next by SEP. */ -static void +void print_switch_values (print_switch_fn_type); + +void print_switch_values (print_switch_fn_type print_fn) { int pos = 0; Index: gcc/varasm.c === --- gcc/varasm.c (revision 249007) +++ gcc/varasm.c (working copy) @@ -7545,6 +7545,32 @@ v.release (); } +/* Print TEXT to asm_out_file if
Re: RFA: Enhance information recorded by -frecord-gcc-switches
Hi Richard, >>The -frecord-gcc-switches option records the gcc command line. It >>does not however expand options like -O2 into the optimizations that >>this enables. > > I think individually enabled passes by -On are not really useful information > as you generally cannot replace -On by a set of other switches on the > command-line. Is that true ? I always thought that -O2 could be duplicated by using a (very long) set of individual command line options. Regardless, the point of this patch is to record which options were enabled, via whatever route, in the binaries. This can be useful to users, or distributors, who want to check that, for example, a specific security option was enabled, or that a particular a particular optimization was run. Cheers Nick
Re: RFA: Enhance information recorded by -frecord-gcc-switches
Hi Jakub, >> Regardless, the point of this patch is to record which options were enabled, >> via >> whatever route, in the binaries. This can be useful to users, or >> distributors, >> who want to check that, for example, a specific security option was enabled, >> or >> that a particular a particular optimization was run. > > And that again doesn't tell you whether the particular optimization pass was > run, just that some flag variable was zero or non-zero or had some other > value. The decisions in the compiler are more complex and keep changing > between compiler versions. For one particular compiler version, -O2 vs. -O1 > if that is what was originally used to compile something is all you need, > that implies a particular behavior, set of options and their interactions. > For comparisons between different compiler versions, some of the options > are ignored, others are added, others change meaning, and expanding the list > of guarded options isn't really useful. OK -so we need some other way of recording what optimization passes were actually run. Fortunately I have something in mind. Patch withdrawn. Cheers Nick
Re: [PATCH 2/2] [MSP430] Fix issues handling .persistent attribute (PR 78818)
Hi Jozef, > Ok for trunk and gcc-7-branch? Approved - please apply (to both). Cheers Nick
Re: [PATCH 2/2] [MSP430] Fix issues handling .persistent attribute (PR 78818)
Hi Jozef, > Sorry, didn't mention in that last post that I don't have write access, > could someone please apply this for me. Applied. Sorry about the delay (again). Cheers Nick
Re: [PATCH 0/3] Add __builtin_load_no_speculate
Hi Guys, It seems to me that it might be worth taking a step back here, and consider adding a security framework to gcc. Mitigations for CVEs in the past have resulted in individual patches being added to gcc, oftern in a target specific manner, and with no real framework to support them, document them, or indicate to an external tool that they have been applied. In addition security fixes often result in the generation of less optimal code, and so it might be useful to have a way to tell other parts of gcc that a given particular sequence should not be altered. Not that I am an expert in this area, but I do think that it is something that should be discussed... Cheers Nick
RFA: Restore ability to build zlib in a srcdir == builddir
Hi Guys, The patch below allows the zlib library to be built when the build directory is the same as the source directory. This patch has been in the binutils/gdb sources for a while now, but unfortunately was never copied to gcc. So I am trying to right that mistake now. OK to apply ? Cheers Nick ./ChangeLog * zlib/configure.ac: Restore old behaviour of only enabling multilibs when a target subdirectory is defined. This allows building with srcdir == builddir. * zlib/configure: Regenerate. diff --git a/zlib/configure.ac b/zlib/configure.ac index fb8d943905..57d6fa56b6 100644 --- a/zlib/configure.ac +++ b/zlib/configure.ac @@ -4,7 +4,9 @@ AC_PREREQ(2.64) AC_INIT AC_CONFIG_SRCDIR([zlib.h]) -AM_ENABLE_MULTILIB(, ..) +if test -n "${with_target_subdir}"; then + AM_ENABLE_MULTILIB(, ..) +fi AC_CANONICAL_SYSTEM
Re: [tree.c] Replace cast to (char *) by const_cast
Hi Prathamesh, > I am getting the following build error with trunk: > ../../gcc/gcc/tree.c: In member function ‘void > escaped_string::escape(const char*)’: > ../../gcc/gcc/tree.c:12457:20: error: cast from type ‘const char*’ to > type ‘char*’ casts away qualifiers [-Werror=cast-qual] >m_str = (char *) unescaped; > ^ > I think this is caused by r261697 in tree.c: > m_str = (char *) unescaped; > > The patch changes it to const_cast (unescaped) which fixes the > build for me. I cannot approve this patch, but I can say thanks very much for catching this problem and proposing a fix. I guess that I must be using an old version of g++ for my testing as this error did not show up. :-( Cheers Nick
Re: RFA: Sanitize deprecation messages (PR 84195)
Hi Martin, > I'm getting a bootstrap failure: *sigh* yes - my bad. Fortunately a patch has already been posted: https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01039.html And I think that it has now been approved. Cheers Nick
Re: [RX] Fix PR 81821
Hi Oleg, > OK for trunk and GCC 7? Approved. Do you have access to the repository, or would you like me to apply the patch for you ? Cheers Nick
Re: [RX] Fix PR 81819
Hi Oleg, > gcc/ChangeLog: > PR target/81819 > * config/rx/rx.c (rx_is_restricted_memory_address): > Handle SUBREG case. Go ahead make my day^H^H^H^H^H^H Approved - please apply. Cheers Nick
RFA: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
Hi H.J. Attached is a potential patch for PR 84145: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145 The problem was that the code to check that the --mibt and/or -mshstk options have been correctly enabled for control flow protection did not take into account that the wrong option might have been enabled. So the patch inverts the test, looking at the value of flag_cf_protection first and then checking to see if the needed x86 specific options have been enabled. This gives results like this: % gcc -c main.c % gcc -c main.c -fcf-protection=full cc1: error: '-fcf-protection=full' requires CET support on this target. Use -mcet or both of -mibt and -mshstk options to enable CET % gcc -c main.c -fcf-protection=full -mcet % gcc -c main.c -fcf-protection=full -mibt cc1: error: '-fcf-protection=full' requires CET support on this target. Use -mcet or both of -mibt and -mshstk options to enable CET % gcc -c main.c -fcf-protection=full -mibt -mshstk % gcc -c main.c -fcf-protection=branch cc1: error: '-fcf-protection=branch' requires CET support on this target. Use -mcet or -mibt to enable CET % gcc -c main.c -fcf-protection=branch -mcet % gcc -c main.c -fcf-protection=branch -mibt % gcc -c main.c -fcf-protection=branch -mshstk cc1: error: '-fcf-protection=branch' requires CET support on this target. Use -mcet or -mibt to enable CET % gcc -c main.c -fcf-protection=return cc1: error: '-fcf-protection=return' requires CET support on this target. Use -mcet or -mshstk to enable CET % gcc -c main.c -fcf-protection=return -mcet % gcc -c main.c -fcf-protection=return -mibt cc1: error: '-fcf-protection=return' requires CET support on this target. Use -mcet or -mshstk to enable CET % gcc -c main.c -fcf-protection=return -mshstk % What do you think ? Is the patch OK for the mainline ? Cheers Nick gcc/ChangeLog 2018-02-05 Nick Clifton PR 84145 * config/i386/i386.c (ix86_option_override_internal): Rework checks for -fcf-protection and -mibt/-mshstk. Index: gcc/config/i386/i386.c === --- gcc/config/i386/i386.c (revision 257389) +++ gcc/config/i386/i386.c (working copy) @@ -4915,30 +4915,43 @@ /* Do not support control flow instrumentation if CET is not enabled. */ if (opts->x_flag_cf_protection != CF_NONE) { - if (!(TARGET_IBT_P (opts->x_ix86_isa_flags2) - || TARGET_SHSTK_P (opts->x_ix86_isa_flags))) + switch (flag_cf_protection) { - if (flag_cf_protection == CF_FULL) + case CF_NONE: + break; + case CF_BRANCH: + if (! TARGET_IBT_P (opts->x_ix86_isa_flags2)) { - error ("%<-fcf-protection=full%> requires CET support " -"on this target. Use -mcet or one of -mibt, " -"-mshstk options to enable CET"); + error ("%<-fcf-protection=branch%> requires CET support " +"on this target. Use -mcet or -mibt to enable CET"); + flag_cf_protection = CF_NONE; + return false; } - else if (flag_cf_protection == CF_BRANCH) + break; + case CF_RETURN: + if (! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) { - error ("%<-fcf-protection=branch%> requires CET support " -"on this target. Use -mcet or one of -mibt, " -"-mshstk options to enable CET"); + error ("%<-fcf-protection=return%> requires CET support " +"on this target. Use -mcet or -mshstk to enable CET"); + flag_cf_protection = CF_NONE; + return false; } - else if (flag_cf_protection == CF_RETURN) + break; + case CF_FULL: + if ( ! TARGET_IBT_P (opts->x_ix86_isa_flags2) +|| ! TARGET_SHSTK_P (opts->x_ix86_isa_flags)) { - error ("%<-fcf-protection=return%> requires CET support " -"on this target. Use -mcet or one of -mibt, " + error ("%<-fcf-protection=full%> requires CET support " +"on this target. Use -mcet or both of -mibt and " "-mshstk options to enable CET"); + flag_cf_protection = CF_NONE; + return false; } - flag_cf_protection = CF_NONE; - return false; + break; + default: + gcc_unreachable (); } + opts->x_flag_cf_protection = (cf_protection_level) (opts->x_flag_cf_protection | CF_SET); }
RFA: Sanitize deprecation messages (PR 84195)
Hi Martin, Hi Dodji, Hi David, PR 84195 makes the point that deprecation messages do not follow the formatting guidelines set out by the -fmessage-length option, and that they could even contain unwanted control characters: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84195 Below is my attempt at a patch to fix this. It is not very clever, just replacing the control characters with spaces, and doing it each time that a deprecation warning is displayed. But I figured that such warnings are going to be rare and do not need to be efficiently handled. So I choose to keep it simple. :-) No regressions with an x86_64-pc-linux-gnu toolchain. OK to apply ? Cheers Nick gcc/ChangeLog 2018-02-05 Nick Clifton PR 84195 * tree.c (warn_deprecated_use): Sanitize deprecation messages. Index: gcc/tree.c === --- gcc/tree.c (revision 257389) +++ gcc/tree.c (working copy) @@ -12436,6 +12436,39 @@ else msg = NULL; + char * new_msg = NULL; + if (msg) +{ + unsigned int i; /* FIXME: Do we need to check to see if msg is longer +than 2^32 ? */ + + /* PR 84195: Sanitize the message: + +If -fmessage-length is set to 0 then replace newline characters with +spaces. Replace other non-printing ASCII characters with spaces too. + +We do this check here, rather than where the deprecated attribute is +recorded because the setting of -fmessage-length can be changed +between those two locations. */ + for (i = 0; i < strlen (msg); i++) + { + char c = msg[i]; + + if (! ISCNTRL (c)) + continue; + + if (c != '\n' || ! pp_is_wrapping_line (global_dc->printer)) + { + if (new_msg == NULL) + new_msg = xstrdup (msg); + new_msg [i] = ' '; + } + } + + if (new_msg) + msg = new_msg; +} + bool w; if (DECL_P (node)) { @@ -12505,6 +12538,8 @@ } } } + + free (new_msg); } /* Return true if REF has a COMPONENT_REF with a bit-field field declaration
Re: PR 84154: Fix checking -mibt and -mshstk options for control flow protection
Hi Igor, >> Attached is a potential patch for PR 84145: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84145 > Coincidentally, I have worked on the same patch. Great minds, etc :-) > Please look at the patch, I uploaded it to the bug. The main differences are > > - updated the output messages to be more informative; > - updated the tests and add couple of new tests to check the messages; > - fixed a typo in the doc file related to fcf-protection; > > I am ok with the changes in i386.c but would like to update the messages. > Could you incorporate my changes and proceed? Or would you like me to finish > the fix? If you are happy to finish the fix then please do so. Your fix is more thorough than mine, so I am happy to see it go on. Although I should say that I am not an x86 maintainer, so I cannot approve it. Cheers Nick
Re: RFA: Sanitize deprecation messages (PR 84195)
Hi Martin, > My only suggestions are to consider how control characters and > excessively messages are handled in other contexts and adopt > the same approach here. Are there other places where user-supplied messages are displayed by gcc ? > In the tests I did, control characters > were mostly escaped (e.g., as \n or as \x0a) rather than replaced > with spaces. Ooo - what tests were these ? I agree that it there is a standard way to handle these characters then the deprecated attribute ought to do the same. (Plus hopefully there is already a function in gcc to do this kind of processing, so the deprecation code can just call it). > I haven't thought too hard about how excessively > long messages should be handled, or about the interactions with > -fmessage-length= (i.e., if messages longer than the limit should > be broken up.) I believe that this will happen automatically. The diagnostic display routine will automatically insert newlines into the output if the message length is non-zero, and the message extends past this limit. (Well provided that the message contains spaces. The newlines are only inserted in place of space characters so a very long, single word message will not be split...) Cheers Nick
Re: PING [PATCH] RX movsicc degrade fix
Hi Sebastian, Sorry for missing this one. If it helps in the future, feel free to ping me directly. > +2018-01-09 Sebastian Perta > + > + *config/rx.md: updated "movsicc" expand to be matched by GCC > + *testsuite/gcc.target/rx/movsicc.c: new test case Approved - please apply. Cheers Nick
Re: PING [PATCH] -mjsr option bug fix
Hi Sebastian, > +2018-01-05 Sebastian Perta > + > + * config/rx/constraints.md: added new constraint > CALL_OP_SYMBOL_REF > + to allow or block "symbol_ref" depending on value of TARGET_JSR > + * config/rx/rx.md: use CALL_OP_SYMBOL_REF in call_internal and > + call_value_internal insns > + Approved - please apply. Note for the future - it is usually best to provide changelog entries as plain text, rather than a context diff, as they almost never apply cleanly... Cheers Nick
Re: RFA: Sanitize deprecation messages (PR 84195)
Hi Martin, Hi David, OK - attached is a new patch that: * Replaces control characters with their escape equivalents. * Includes a testcase. I was not sure what to do about the inconsistencies between the behaviour of #warning and #pragma GCC warning, (and the error equivalents), so I decided to leave it for now. Fixing them will require mucking about in the C preprocessor library, which I did not fancy doing at the moment. So, is this enhanced patch OK now ? Cheers Nick gcc/ChangeLog 2018-02-07 Nick Clifton PR 84195 * tree.c (warn_deprecated_use): Replace control characters in deprecation messages with escape sequences. gcc/testsuite/ChangeLog 2018-02-07 Nick Clifton * gcc.c-torture/compile/pr84195.c: New test. pr84195.patch.2 Description: Unix manual page
Re: RFA: Sanitize deprecation messages (PR 84195)
Hi David, >> + /* PR 84195: Replace control characters in the message with their >> + escaped equivalents. Allow newlines if -fmessage-length has >> + been set to a non-zero value. > > I'm not quite sure why we allow newlines in this case, sorry. Because the documentation for -fmessage-length says: Try to format error messages so that they fit on lines of about N characters. If N is zero, then no line-wrapping is done; each error message appears on a single line. This is the default for all front ends. So with a non-zero message length, multi-line messages are allowed. At least that was my understanding of the option. Thanks for the patch review. I will get onto fixing the points you raised today. Cheers Nick
RFA: Fix PR 68028: LTO error when compiling PowerPC binaries with single precision floating point
Hi Segher, OK, here is an official submission of my patch to fix PR 68028. I should note that Richard Guenther feels that there is a better way to solve the problem - by only initializing the values once - but I still like my solution, so I am offering it here. OK to apply ? Cheers Nick gcc/ChangeLog 2018-02-08 Nick Clifton * config/rs6000/rs6000.c (rs6000_option_override_internal): In LTO mode prefer function attributes over command line -mcpu setting. Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 257282) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -4834,12 +4834,25 @@ if (main_target_opt) { - if (main_target_opt->x_rs6000_single_float != rs6000_single_float) - error ("target attribute or pragma changes single precision floating " - "point"); - if (main_target_opt->x_rs6000_double_float != rs6000_double_float) - error ("target attribute or pragma changes double precision floating " - "point"); + /* PR 68028: In LTO mode the -mcpu value is passed in as a function +attribute rather than on the command line. So instead of checking +for discrepancioes, we enforce the choice determined by the +attributes. */ +if (in_lto_p) + { +rs6000_single_float = main_target_opt->x_rs6000_single_float; +rs6000_double_float = main_target_opt->x_rs6000_double_float; + } +/* There could be an 'else' statement here but it is hardly worth + it as the compiler will make the optimization anyway, and this + way we avoid indenting the code unnecessarily. */ + +if (main_target_opt->x_rs6000_single_float != rs6000_single_float) + error ("target attribute or pragma changes single precision floating " + "point"); +if (main_target_opt->x_rs6000_double_float != rs6000_double_float) + error ("target attribute or pragma changes double precision floating " + "point"); } rs6000_always_hint = (rs6000_tune != PROCESSOR_POWER4
Re: RFA: Fix PR 68028: LTO error when compiling PowerPC binaries with single precision floating point
Hi Segher, > I thought you were going to do a patch like the following, to make the > e500 cores less special (they are not): Sorry - my bad. I defer to your patch then. Whatever it takes to get the BZ fixed and off the books... :-) Cheers Nick
Re: RFA: Sanitize deprecation messages (PR 84195)
Hi David, Hi Martin, OK, take 3 of the patch is attached. In this version: * The escaping is handled by a new class. * Self-tests have been added (and they helped find a bug - yay!) * The documentation has been extended to mention -fmessage-length's effect on the #-directives, and to add documentation for #pragma GCC error and #pragma GCC warning. (Which appeared to be missing). I did try to add backslash characters to the regexp in the new testcase but I just could not find a way to persuade dejagnu to accept them. OK to apply ? (Apologies for the poor C++ coding - it is not my strong point. I am an assembler level programmer at heart). Cheers Nick gcc/ChangeLog 2018-02-09 Nick Clifton PR 84195 * tree.c (escaped_string): New class. Converts an unescaped string into its escaped equivalent. (warn_deprecated_use): Use the new class to convert the deprecation message, if present. (test_escaped_strings): New self test. (test_c_tests): Add test_escaped_strings. gcc/testsuite/ChangeLog 2018-02-07 Nick Clifton * gcc.c-torture/compile/pr84195.c: New test. pr84195.patch.3 Description: Unix manual page
Re: [RX] Fix PR 83831 -- Unused bclr, bnot, bset insns
Hi Oleg, > OK for trunk? > gcc/ChangeLog: > > PR target/83831 > * config/rx/rx-protos.h (rx_reg_dead_or_unused_after_insn, > rx_copy_reg_dead_or_unused_notes, rx_fuse_in_memory_bitop): New > declarations. > (set_of_reg): New struct. > (rx_find_set_of_reg, rx_find_use_of_reg): New functions. > * config/rx/rx.c (rx_reg_dead_or_unused_after_insn, > rx_copy_reg_dead_or_unused_notes, rx_fuse_in_memory_bitop): New > functions. > * config/rx/rx.md (andsi3, iorsi3, xorsi3): Convert to insn_and_split. > Split into bitclr, bitset, bitinvert patterns if appropriate. > (*bitset, *bitinvert, *bitclr): Convert to named insn_and_split and > use rx_fuse_in_memory_bitop. > (*bitset_in_memory, *bitinvert_in_memory, *bitclr_in_memory): Convert > to named insn, correct maximum insn length. > > gcc/testsuite/ChangeLog: > > PR target/83831 > * gcc.target/rx/pr83831.c: New tests. Approved - please apply - and thanks very much for doing this! Cheers Nick
Re: RFA: Sanitize deprecation messages (PR 84195)
Hi David, Attached is a revised version of the patch which I hope addresses all of your (very helpful) comments on the v3 patch. OK to apply once the sources are back on stage 1 ? Cheers Nick gcc/ChangeLog 2018-02-09 Nick Clifton PR 84195 * tree.c (escaped_string): New class. Converts an unescaped string into its escaped equivalent. (warn_deprecated_use): Use the new class to convert the deprecation message, if present. (test_escaped_strings): New self test. (test_c_tests): Add test_escaped_strings. * doc/extend.texi (deprecated): Add a note that the deprecation message is affected by the -fmessage-length option, and that control characters will be escaped. (#pragma GCC error): Document this pragma. (#pragma GCC warning): Likewise. * doc/invoke.texi (-fmessage-length): Document this option's effect on the #warning and #error preprocessor directives and the deprecated attribute. gcc/testsuite/ChangeLog 2018-02-09 Nick Clifton PR 84195 * gcc.c-torture/compile/pr84195.c: New test. pr84195.patch.4 Description: Unix manual page
Re: RFA: Sanitize deprecation messages (PR 84195)
Hi Martin, > Since the class manages a resource it should ideally make sure it > doesn't try to release the same resource multiple times. I.e., its > copy constructor and assignment operators should either "do the right > thing" (whatever you think that is) or be made inaccessible (or declared > deleted in C++ 11). > > For example: > > { > escaped_string a; > a.escape ("foo\nbar"); > > escaped_string b (a); > // b destroys its m_str > // double free in a's destructor here > } I am not sure that this can happen. First of the escaped_string class does not have constructor that accepts a char* argument. (Maybe in C++ this is done automatically ? My C++-fu is very weak). Secondly the m_owned private field is only set to true when the m_str field is set to a string allocated by the particular instance of the class, and memory is only freed by the destructor if m_owned is true. So even this should work: { escaped_string a,b; a.escape ("foo\nbar"); b.escape ((const char *) a); } The destructor for B will not free any memory, even though its m_str field has been set to the same string as A, because its m_owned field will be set to FALSE. Cheers Nick
Re: plugin-api.h patch to add a new interface for linker plugins
Hi Cary, Hi Sriraman, >> Ping. Is this alright to apply now or should I wait for Stage 1? >> >> * plugin-api.h (ld_plugin_get_wrap_symbols): New >> plugin interface. > > I'd say go ahead and apply the patch in binutils, and wait for Stage 1 > to sync back to GCC, unless someone there OKs it sooner. > > Nick, is that OK? Yes, that is fine with me. Cheers Nick
Re: [PATCH] RX TARGET_RTX_COSTS function
Hi Oleg, > Ping. Sorry - I am not very good at spotting RX bugs on the gcc-patches list. :-( >> gcc/ChangeLog: >> * config/rx/rx.c (rx_rtx_costs): New function. >> (TARGET_RTX_COSTS): Override to use rx_rtx_costs. Approved - please apply. Cheers Nick