public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c/109650] New: avr-gcc incorrect code with -Os
@ 2023-04-27 17:13 thierer at web dot de
  2023-04-27 17:26 ` [Bug target/109650] " pinskia at gcc dot gnu.org
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: thierer at web dot de @ 2023-04-27 17:13 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

            Bug ID: 109650
           Summary: avr-gcc incorrect code with -Os
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: thierer at web dot de
  Target Milestone: ---

Created attachment 54942
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54942&action=edit
preprocessed input file

The attached file imho creates invalid code with avr-gcc 12.2.0 when compiling
with "-Os". The same code works with either different optimization settings or
avr-gcc 11.3.0 (haven't tried any other versions).

The misbehaving code is this function:

static bool test_func(uint8_t p1, uint8_t p2) {
  if (p1 == 0 || p1 > 7)
    return false;

  if      (p1 < 3) return p2 <= 8;
  else if (p1 < 5) return p2 <= 6;
  else if (p1 < 7) return p2 <= 4;
  else             return p2 <= 2;
}

It should return false for all values of p2 if p1 is 0 or > 7 and the result
should depend on p2 for inbetween values of p1.

The code contains a lot of boilerplate for sending a showcase output over
USART0. This is the result for values of 0 <= p2 < 10 (columns) for 0 < p1 < 9
(rows):

  0123456789
0 ??????????
1 XXXXXXXXX?
2 XXXXXXXXX?
3 XXXXXX????
4 XXXXXX????
5 XXXX??????
6 XXXX??????
7 XX????????
8 ??????????

"X" means test_func() returned true, "?" false. The result for p1 in [0,1,2,8]
is correct, all the other results are off (too low) by 1. For example, for p1
== 3 the function should return true for all p2 <= 6, but it only does for <=
5.

I'm not too familiar with AVR assembly, but the problem seems to be that the
comparisons that calculate the result value all use the same brlo (branch if
lower) instruction at .L34, but the compiler fails to compensate for the
"lower" instead of "lower or equal" for all but the first (<= 8) condition:

.L21:
        cpi r17,lo8(7)
        brsh .L25
        cpi r28,lo8(3)
        brsh .L12
        cpi r29,lo8(9)          ; this correctly compares to 9 == 8+1
.L34:
        brlo .L27
.L25:
        ldi r24,lo8(63)
.L11:

[...]

.L12:
        cpi r28,lo8(5)
        brsh .L15
        cpi r29,lo8(6)          ; but this does not (should be 6+1 == 7)
        rjmp .L34
.L15:
        cpi r28,lo8(7)
        breq .L17
        cpi r29,lo8(4)          ; neither does this (should be 4+1 == 5)
        rjmp .L34
.L17:
        cpi r29,lo8(2)          ; nor this (should be 2+1 == 3)
        rjmp .L34
.L27:


Tested with the respective Arch Linux x86_64 avr-gcc packages.

Output of "avr-gcc -v":

> avr-gcc -v -save-temps -Wall -Wextra -mmcu=atmega1284p -Os --param=min-pagesize=0 avr-bug.c
Using built-in specs.
Reading specs from /usr/lib/gcc/avr/12.2.0/device-specs/specs-atmega1284p
COLLECT_GCC=avr-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/avr/12.2.0/lto-wrapper
Target: avr
Configured with: /build/avr-gcc/src/gcc-12.2.0/configure
--disable-install-libiberty --disable-libssp --disable-libstdcxx-pch
--disable-libunwind-exceptions --disable-linker-build-id --disable-nls
--disable-werror --disable-__cxa_atexit --enable-checking=release
--enable-clocale=gnu --enable-gnu-unique-object --enable-gold
--enable-languages=c,c++ --enable-ld=default --enable-lto --enable-plugin
--enable-shared --infodir=/usr/share/info --libdir=/usr/lib
--libexecdir=/usr/lib --mandir=/usr/share/man --prefix=/usr --target=avr
--with-as=/usr/bin/avr-as --with-gnu-as --with-gnu-ld --with-ld=/usr/bin/avr-ld
--with-plugin-ld=ld.gold --with-system-zlib --with-isl
--enable-gnu-indirect-function
Thread model: single
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.0 (GCC)
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra'  '-Os'
'--param=min-pagesize=0' '-mdouble=32' '-mlong-double=64'
'-specs=device-specs/specs-atmega1284p' '-mmcu=avr51' '-dumpdir' 'a-'
 /usr/lib/gcc/avr/12.2.0/cc1 -E -quiet -v -imultilib avr51
-D__AVR_ATmega1284P__ -D__AVR_DEVICE_NAME__=atmega1284p avr-bug.c -mn-flash=2
-mno-skip-bug -mdouble=32 -mlong-double=64 -mmcu=avr51 -Wall -Wextra -Os
-fpch-preprocess -o a-avr-bug.i
ignoring nonexistent directory
"/usr/lib/gcc/avr/12.2.0/../../../../avr/sys-include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/avr/12.2.0/include
 /usr/lib/gcc/avr/12.2.0/include-fixed
 /usr/lib/gcc/avr/12.2.0/../../../../avr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra'  '-Os'
'--param=min-pagesize=0' '-mdouble=32' '-mlong-double=64'
'-specs=device-specs/specs-atmega1284p' '-mmcu=avr51' '-dumpdir' 'a-'
 /usr/lib/gcc/avr/12.2.0/cc1 -fpreprocessed a-avr-bug.i -mn-flash=2
-mno-skip-bug -quiet -dumpdir a- -dumpbase avr-bug.c -dumpbase-ext .c
-mdouble=32 -mlong-double=64 -mmcu=avr51 -Os -Wall -Wextra -version
--param=min-pagesize=0 -o a-avr-bug.s
GNU C17 (GCC) version 12.2.0 (avr)
        compiled by GNU C version 12.1.1 20220730, GMP version 6.2.1, MPFR
version 4.1.0-p13, MPC version 1.2.1, isl version isl-0.26-GMP

warning: MPFR header version 4.1.0-p13 differs from library version 4.2.0.
warning: MPC header version 1.2.1 differs from library version 1.3.1.
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C17 (GCC) version 12.2.0 (avr)
        compiled by GNU C version 12.1.1 20220730, GMP version 6.2.1, MPFR
version 4.1.0-p13, MPC version 1.2.1, isl version isl-0.26-GMP

warning: MPFR header version 4.1.0-p13 differs from library version 4.2.0.
warning: MPC header version 1.2.1 differs from library version 1.3.1.
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 00c69299beaa7f88846ded0b751d0e18
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra'  '-Os'
'--param=min-pagesize=0' '-mdouble=32' '-mlong-double=64'
'-specs=device-specs/specs-atmega1284p' '-mmcu=avr51' '-dumpdir' 'a-'
 /usr/bin/avr-as -v -mmcu=avr51 -mgcc-isr -mno-skip-bug -o a-avr-bug.o
a-avr-bug.s
GNU assembler version 2.40 (avr) using BFD version (GNU Binutils) 2.40
COMPILER_PATH=/usr/lib/gcc/avr/12.2.0/:/usr/lib/gcc/avr/12.2.0/:/usr/lib/gcc/avr/:/usr/lib/gcc/avr/12.2.0/:/usr/lib/gcc/avr/
LIBRARY_PATH=/usr/lib/gcc/avr/12.2.0/avr51/:/usr/lib/gcc/avr/12.2.0/../../../../avr/lib/avr51/:/usr/lib/gcc/avr/12.2.0/:/usr/lib/gcc/avr/12.2.0/../../../../avr/lib/
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra'  '-Os'
'--param=min-pagesize=0' '-mdouble=32' '-mlong-double=64'
'-specs=device-specs/specs-atmega1284p' '-mmcu=avr51' '-dumpdir' 'a.'
 /usr/lib/gcc/avr/12.2.0/collect2 -plugin
/usr/lib/gcc/avr/12.2.0/liblto_plugin.so
-plugin-opt=/usr/lib/gcc/avr/12.2.0/lto-wrapper -plugin-opt=-fresolution=a.res
-plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lm
-plugin-opt=-pass-through=-lc -plugin-opt=-pass-through=-latmega1284p -mavr51
-Tdata 0x800100
/usr/lib/gcc/avr/12.2.0/../../../../avr/lib/avr51/crtatmega1284p.o
-L/usr/lib/gcc/avr/12.2.0/avr51
-L/usr/lib/gcc/avr/12.2.0/../../../../avr/lib/avr51 -L/usr/lib/gcc/avr/12.2.0
-L/usr/lib/gcc/avr/12.2.0/../../../../avr/lib a-avr-bug.o --start-group -lgcc
-lm -lc -latmega1284p --end-group
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-Wall' '-Wextra'  '-Os'
'--param=min-pagesize=0' '-mdouble=32' '-mlong-double=64'
'-specs=device-specs/specs-atmega1284p' '-mmcu=avr51' '-dumpdir' 'a.'

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
@ 2023-04-27 17:26 ` pinskia at gcc dot gnu.org
  2023-04-28 15:09 ` gjl at gcc dot gnu.org
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-04-27 17:26 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
x86_64 prints (with a simple: `#define uart_putc __builtin_putchar`):
```
  0123456789
0 ??????????
1 XXXXXXXXX?
2 XXXXXXXXX?
3 XXXXXXX???
4 XXXXXXX???
5 XXXXX?????
6 XXXXX?????
7 XXX???????
8 ??????????
```
Which I think is the correct output

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
  2023-04-27 17:26 ` [Bug target/109650] " pinskia at gcc dot gnu.org
@ 2023-04-28 15:09 ` gjl at gcc dot gnu.org
  2023-04-28 15:13 ` gjl at gcc dot gnu.org
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-04-28 15:09 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gjl at gcc dot gnu.org

--- Comment #2 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Created attachment 54953
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54953&action=edit
Full C test case that doesn't require hardware like UART.

This testcase tests against the code as generated by avr-gcc-8.5 -Os. It should
work for all AVR devices including reduced tiny like ATtiny40, thus eligible
for

testsuite/gcc.target/avr/torture

Pairs that produce different results with avr-gcc v14 (current master). are:

p1,p2 = 3,6;  4,6;  5,4;  6,4  and  7,2.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
  2023-04-27 17:26 ` [Bug target/109650] " pinskia at gcc dot gnu.org
  2023-04-28 15:09 ` gjl at gcc dot gnu.org
@ 2023-04-28 15:13 ` gjl at gcc dot gnu.org
  2023-04-28 15:14 ` [Bug other/109650] " gjl at gcc dot gnu.org
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-04-28 15:13 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-04-28
             Status|UNCONFIRMED                 |NEW

--- Comment #3 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Confirmed with "gcc version 14.0.0 20230420 (experimental) (GCC)"

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug other/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (2 preceding siblings ...)
  2023-04-28 15:13 ` gjl at gcc dot gnu.org
@ 2023-04-28 15:14 ` gjl at gcc dot gnu.org
  2023-04-29  9:05 ` [Bug middle-end/109650] " gjl at gcc dot gnu.org
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-04-28 15:14 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |other

--- Comment #4 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Set component to "other" until we know what component actually causes this bug.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug middle-end/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (3 preceding siblings ...)
  2023-04-28 15:14 ` [Bug other/109650] " gjl at gcc dot gnu.org
@ 2023-04-29  9:05 ` gjl at gcc dot gnu.org
  2023-05-02 16:46 ` gjl at gcc dot gnu.org
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-04-29  9:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

--- Comment #5 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Created attachment 54956
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54956&action=edit
pr109650-2.c: A simpler C test case without assembly.

Again, this testcase triggers with avr-gcc -Os:

typedef _Bool bool;
typedef __UINT8_TYPE__ uint8_t;

static inline __attribute__((__always_inline__))
bool func1 (uint8_t p1, uint8_t p2)
{
    if (p1)
        return p2 <= 8;
    return p2 <= 2;
}

__attribute__((__noinline__, __noclone__))
bool func2 (uint8_t p1, uint8_t p2)
{
    return func1 (p1, p2);
}

int main (void)
{
    if (func1 (0, 1) != func2 (0, 1)) __builtin_abort();
    if (func1 (0, 2) != func2 (0, 2)) __builtin_abort();
    if (func1 (0, 3) != func2 (0, 3)) __builtin_abort();

    if (func1 (1, 7) != func2 (1, 7)) __builtin_abort();
    if (func1 (1, 8) != func2 (1, 8)) __builtin_abort();
    if (func1 (1, 9) != func2 (1, 9)) __builtin_abort();

    __builtin_exit (0);
}

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug middle-end/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (4 preceding siblings ...)
  2023-04-29  9:05 ` [Bug middle-end/109650] " gjl at gcc dot gnu.org
@ 2023-05-02 16:46 ` gjl at gcc dot gnu.org
  2023-05-03 16:57 ` [Bug target/109650] " gjl at gcc dot gnu.org
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-05-02 16:46 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |saaadhu at gcc dot gnu.org

--- Comment #6 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
As it appears, disabling avr.cc's machine dependent reorg by means of
-fdisable-rtl-mach can stop this PR.  Maybe avr.cc::avr_reorg() is no more
correct after PR92729 because contrary to cc0, now a branch instruction may
have more than one CCmode setter.  A part of this optimization could be run
prior to .split2 when cbranch insns are still intact.  In particular
transformations like from "if (x <= 2)" to "if (x < 3)" must be performed on
cbranch, or otherwise avr_reorg() must check that there is only one CCmode
setter, or otherwise adjust *all* CCmode setters.

This optimization should not run too early so that
avr_reorg_remove_redundant_compare can still do its job on switch-case if-else
trees.

I am allowing me to add Senthil to CC, maybe he has some ideas.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (5 preceding siblings ...)
  2023-05-02 16:46 ` gjl at gcc dot gnu.org
@ 2023-05-03 16:57 ` gjl at gcc dot gnu.org
  2023-05-19  8:31 ` gjl at gcc dot gnu.org
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-05-03 16:57 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |target

--- Comment #7 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Target issue: Implicitly introduced by PR92729 as avr's machine dependent reorg
in avr.cc::avr_reorg() still assumes there is nothing between a comparison and
the following conditional branch, like it was with cc0. However, pass "jump2"
introduces a label between a comarison and a branch insn.

Affected are all versions from v12 onwards.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (6 preceding siblings ...)
  2023-05-03 16:57 ` [Bug target/109650] " gjl at gcc dot gnu.org
@ 2023-05-19  8:31 ` gjl at gcc dot gnu.org
  2023-05-19  8:52 ` gjl at gcc dot gnu.org
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-05-19  8:31 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

--- Comment #8 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Created attachment 55115
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55115&action=edit
Proposed patch.

This is a proposed patch to fix this PR.  Under review at

https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618973.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (7 preceding siblings ...)
  2023-05-19  8:31 ` gjl at gcc dot gnu.org
@ 2023-05-19  8:52 ` gjl at gcc dot gnu.org
  2023-06-10 19:59 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-05-19  8:52 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

--- Comment #9 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
(In reply to Georg-Johann Lay from comment #8)
> Created attachment 55115 [details]
> Proposed patch.
> 
> This is a proposed patch to fix this PR.  Under review at

Here actually:

https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618976.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (8 preceding siblings ...)
  2023-05-19  8:52 ` gjl at gcc dot gnu.org
@ 2023-06-10 19:59 ` cvs-commit at gcc dot gnu.org
  2023-06-10 20:25 ` cvs-commit at gcc dot gnu.org
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-10 19:59 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Georg-Johann Lay <gjl@gcc.gnu.org>:

https://gcc.gnu.org/g:30a8771c0f5ddcbc329408c3bbf4f100b01acca9

commit r14-1688-g30a8771c0f5ddcbc329408c3bbf4f100b01acca9
Author: Georg-Johann Lay <avr@gjlay.de>
Date:   Sat Jun 10 21:47:53 2023 +0200

    target/109650: Fix wrong code after cc0 -> CCmode transition.

    This patch fixes a wrong-code bug in the wake of PR92729, the transition
that
    turned the AVR backend from cc0 to CCmode.  In cc0, the insn that uses cc0
like
    a conditional branch always follows the cc0 setter, which is no more the
case
    with CCmode where set and use of REG_CC might be in different basic blocks.

    This patch removes the machine-dependent reorg pass in avr_reorg entirely.

    It is replaced by a new, AVR specific mini-pass that runs prior to split2.
    Canonicalization of comparisons away from the "difficult" codes GT[U] and
LE[U]
    is now mostly performed by implementing TARGET_CANONICALIZE_COMPARISON.

    Moreover:

    * Text peephole conditions get "dead_or_set_regno_p (*, REG_CC)" as needed.

    * RTL peephole conditions get "peep2_regno_dead_p (*, REG_CC)" as needed.

    * Conditional branches no more clobber REG_CC.

    * insn output for compares looks ahead to determine the branch mode in use.
      This needs also "dead_or_set_regno_p (*, REG_CC)".

    * Add RTL peepholes for decrement-and-branch detection.

    * Some of the patterns like "*cmphi.zero-extend.0" lost their
      combine-ational part wit PR92729.  Restore them.

    Finally, it fixes some of the many indentation glitches left over from
PR92729.

    gcc/
            PR target/109650
            PR target/92729
            * config/avr/avr-passes.def (avr_pass_ifelse): Insert new pass.
            * config/avr/avr.cc (avr_pass_ifelse): New RTL pass.
            (avr_pass_data_ifelse): New pass_data for it.
            (make_avr_pass_ifelse, avr_redundant_compare, avr_cbranch_cost)
            (avr_canonicalize_comparison, avr_out_plus_set_ZN)
            (avr_out_cmp_ext): New functions.
            (compare_condtition): Make sure REG_CC dies in the branch insn.
            (avr_rtx_costs_1): Add computation of cbranch costs.
            (avr_adjust_insn_length) [ADJUST_LEN_ADD_SET_ZN,
ADJUST_LEN_CMP_ZEXT]:
            [ADJUST_LEN_CMP_SEXT]Handle them.
            (TARGET_CANONICALIZE_COMPARISON): New define.
            (avr_simplify_comparison_p, compare_diff_p, avr_compare_pattern)
            (avr_reorg_remove_redundant_compare, avr_reorg): Remove functions.
            (TARGET_MACHINE_DEPENDENT_REORG): Remove define.
            * config/avr/avr-protos.h (avr_simplify_comparison_p): Remove
proto.
            (make_avr_pass_ifelse, avr_out_plus_set_ZN, cc_reg_rtx)
            (avr_out_cmp_zext): New Protos
            * config/avr/avr.md (branch, difficult_branch): Don't split insns.
            (*cbranchhi.zero-extend.0", *cbranchhi.zero-extend.1")
            (*swapped_tst<mode>, *add.for.eqne.<mode>): New insns.
            (*cbranch<mode>4): Rename to cbranch<mode>4_insn.
            (define_peephole): Add dead_or_set_regno_p(insn,REG_CC) as needed.
            (define_deephole2): Add peep2_regno_dead_p(*,REG_CC) as needed.
            Add new RTL peepholes for decrement-and-branch and
*swapped_tst<mode>.
            Rework signtest-and-branch peepholes for *sbrx_branch<mode>.
            (adjust_len) [add_set_ZN, cmp_zext]: New.
            (QIPSI): New mode iterator.
            (ALLs1, ALLs2, ALLs4, ALLs234): New mode iterators.
            (gelt): New code iterator.
            (gelt_eqne): New code attribute.
            (rvbranch, *rvbranch, difficult_rvbranch, *difficult_rvbranch)
            (branch_unspec, *negated_tst<mode>, *reversed_tst<mode>)
            (*cmpqi_sign_extend): Remove insns.
            (define_c_enum "unspec") [UNSPEC_IDENTITY]: Remove.
            * config/avr/avr-dimode.md (cbranch<mode>4): Canonicalize
comparisons.
            * config/avr/predicates.md (scratch_or_d_register_operand): New.
            * config/avr/constraints.md (Yxx): New constraint.

    gcc/testsuite/
            PR target/109650
            * gcc.target/avr/torture/pr109650-1.c: New test.
            * gcc.target/avr/torture/pr109650-2.c: New test.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (9 preceding siblings ...)
  2023-06-10 19:59 ` cvs-commit at gcc dot gnu.org
@ 2023-06-10 20:25 ` cvs-commit at gcc dot gnu.org
  2023-06-10 20:46 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-10 20:25 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Georg-Johann Lay
<gjl@gcc.gnu.org>:

https://gcc.gnu.org/g:6165b233ecc637efb5edcad8a34aae74b165a711

commit r13-7436-g6165b233ecc637efb5edcad8a34aae74b165a711
Author: Georg-Johann Lay <avr@gjlay.de>
Date:   Sat Jun 10 21:47:53 2023 +0200

    target/109650: Fix wrong code after cc0 -> CCmode transition.

    This patch fixes a wrong-code bug in the wake of PR92729, the transition
that
    turned the AVR backend from cc0 to CCmode.  In cc0, the insn that uses cc0
like
    a conditional branch always follows the cc0 setter, which is no more the
case
    with CCmode where set and use of REG_CC might be in different basic blocks.

    This patch removes the machine-dependent reorg pass in avr_reorg entirely.

    It is replaced by a new, AVR specific mini-pass that runs prior to split2.
    Canonicalization of comparisons away from the "difficult" codes GT[U] and
LE[U]
    is now mostly performed by implementing TARGET_CANONICALIZE_COMPARISON.

    Moreover:

    * Text peephole conditions get "dead_or_set_regno_p (*, REG_CC)" as needed.

    * RTL peephole conditions get "peep2_regno_dead_p (*, REG_CC)" as needed.

    * Conditional branches no more clobber REG_CC.

    * insn output for compares looks ahead to determine the branch mode in use.
      This needs also "dead_or_set_regno_p (*, REG_CC)".

    * Add RTL peepholes for decrement-and-branch detection.

    * Some of the patterns like "*cmphi.zero-extend.0" lost their
      combine-ational part wit PR92729.  Restore them.

    Finally, it fixes some of the many indentation glitches left over from
PR92729.

    gcc/
            PR target/109650
            PR target/92729
            Backport from 2023-05-10 master r14-1688.
            * config/avr/avr-passes.def (avr_pass_ifelse): Insert new pass.
            * config/avr/avr.cc (avr_pass_ifelse): New RTL pass.
            (avr_pass_data_ifelse): New pass_data for it.
            (make_avr_pass_ifelse, avr_redundant_compare, avr_cbranch_cost)
            (avr_canonicalize_comparison, avr_out_plus_set_ZN)
            (avr_out_cmp_ext): New functions.
            (compare_condtition): Make sure REG_CC dies in the branch insn.
            (avr_rtx_costs_1): Add computation of cbranch costs.
            (avr_adjust_insn_length) [ADJUST_LEN_ADD_SET_ZN,
ADJUST_LEN_CMP_ZEXT]:
            [ADJUST_LEN_CMP_SEXT]Handle them.
            (TARGET_CANONICALIZE_COMPARISON): New define.
            (avr_simplify_comparison_p, compare_diff_p, avr_compare_pattern)
            (avr_reorg_remove_redundant_compare, avr_reorg): Remove functions.
            (TARGET_MACHINE_DEPENDENT_REORG): Remove define.
            * config/avr/avr-protos.h (avr_simplify_comparison_p): Remove
proto.
            (make_avr_pass_ifelse, avr_out_plus_set_ZN, cc_reg_rtx)
            (avr_out_cmp_zext): New Protos
            * config/avr/avr.md (branch, difficult_branch): Don't split insns.
            (*cbranchhi.zero-extend.0", *cbranchhi.zero-extend.1")
            (*swapped_tst<mode>, *add.for.eqne.<mode>): New insns.
            (*cbranch<mode>4): Rename to cbranch<mode>4_insn.
            (define_peephole): Add dead_or_set_regno_p(insn,REG_CC) as needed.
            (define_deephole2): Add peep2_regno_dead_p(*,REG_CC) as needed.
            Add new RTL peepholes for decrement-and-branch and
*swapped_tst<mode>.
            Rework signtest-and-branch peepholes for *sbrx_branch<mode>.
            (adjust_len) [add_set_ZN, cmp_zext]: New.
            (QIPSI): New mode iterator.
            (ALLs1, ALLs2, ALLs4, ALLs234): New mode iterators.
            (gelt): New code iterator.
            (gelt_eqne): New code attribute.
            (rvbranch, *rvbranch, difficult_rvbranch, *difficult_rvbranch)
            (branch_unspec, *negated_tst<mode>, *reversed_tst<mode>)
            (*cmpqi_sign_extend): Remove insns.
            (define_c_enum "unspec") [UNSPEC_IDENTITY]: Remove.
            * config/avr/avr-dimode.md (cbranch<mode>4): Canonicalize
comparisons.
            * config/avr/predicates.md (scratch_or_d_register_operand): New.
            * config/avr/constraints.md (Yxx): New constraint.

    gcc/testsuite/
            PR target/109650
            Backport from 2023-05-10 master r14-1688.
            * gcc.target/avr/torture/pr109650-1.c: New test.
            * gcc.target/avr/torture/pr109650-2.c: New test.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (10 preceding siblings ...)
  2023-06-10 20:25 ` cvs-commit at gcc dot gnu.org
@ 2023-06-10 20:46 ` cvs-commit at gcc dot gnu.org
  2023-06-10 21:08 ` gjl at gcc dot gnu.org
  2023-06-10 21:13 ` gjl at gcc dot gnu.org
  13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-10 20:46 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Georg-Johann Lay
<gjl@gcc.gnu.org>:

https://gcc.gnu.org/g:ee92dc2dae45acc79d4dc08ea31adf894831840a

commit r12-9691-gee92dc2dae45acc79d4dc08ea31adf894831840a
Author: Georg-Johann Lay <avr@gjlay.de>
Date:   Sat Jun 10 21:47:53 2023 +0200

    target/109650: Fix wrong code after cc0 -> CCmode transition.

    This patch fixes a wrong-code bug in the wake of PR92729, the transition
that
    turned the AVR backend from cc0 to CCmode.  In cc0, the insn that uses cc0
like
    a conditional branch always follows the cc0 setter, which is no more the
case
    with CCmode where set and use of REG_CC might be in different basic blocks.

    This patch removes the machine-dependent reorg pass in avr_reorg entirely.

    It is replaced by a new, AVR specific mini-pass that runs prior to split2.
    Canonicalization of comparisons away from the "difficult" codes GT[U] and
LE[U]
    is now mostly performed by implementing TARGET_CANONICALIZE_COMPARISON.

    Moreover:

    * Text peephole conditions get "dead_or_set_regno_p (*, REG_CC)" as needed.

    * RTL peephole conditions get "peep2_regno_dead_p (*, REG_CC)" as needed.

    * Conditional branches no more clobber REG_CC.

    * insn output for compares looks ahead to determine the branch mode in use.
      This needs also "dead_or_set_regno_p (*, REG_CC)".

    * Add RTL peepholes for decrement-and-branch detection.

    * Some of the patterns like "*cmphi.zero-extend.0" lost their
      combine-ational part wit PR92729.  Restore them.

    Finally, it fixes some of the many indentation glitches left over from
PR92729.

    gcc/
            PR target/109650
            PR target/92729
            Backport from 2023-05-10 master r14-1688.
            * config/avr/avr-passes.def (avr_pass_ifelse): Insert new pass.
            * config/avr/avr.cc (avr_pass_ifelse): New RTL pass.
            (avr_pass_data_ifelse): New pass_data for it.
            (make_avr_pass_ifelse, avr_redundant_compare, avr_cbranch_cost)
            (avr_canonicalize_comparison, avr_out_plus_set_ZN)
            (avr_out_cmp_ext): New functions.
            (compare_condtition): Make sure REG_CC dies in the branch insn.
            (avr_rtx_costs_1): Add computation of cbranch costs.
            (avr_adjust_insn_length) [ADJUST_LEN_ADD_SET_ZN,
ADJUST_LEN_CMP_ZEXT]:
            [ADJUST_LEN_CMP_SEXT]Handle them.
            (TARGET_CANONICALIZE_COMPARISON): New define.
            (avr_simplify_comparison_p, compare_diff_p, avr_compare_pattern)
            (avr_reorg_remove_redundant_compare, avr_reorg): Remove functions.
            (TARGET_MACHINE_DEPENDENT_REORG): Remove define.
            * config/avr/avr-protos.h (avr_simplify_comparison_p): Remove
proto.
            (make_avr_pass_ifelse, avr_out_plus_set_ZN, cc_reg_rtx)
            (avr_out_cmp_zext): New Protos
            * config/avr/avr.md (branch, difficult_branch): Don't split insns.
            (*cbranchhi.zero-extend.0", *cbranchhi.zero-extend.1")
            (*swapped_tst<mode>, *add.for.eqne.<mode>): New insns.
            (*cbranch<mode>4): Rename to cbranch<mode>4_insn.
            (define_peephole): Add dead_or_set_regno_p(insn,REG_CC) as needed.
            (define_deephole2): Add peep2_regno_dead_p(*,REG_CC) as needed.
            Add new RTL peepholes for decrement-and-branch and
*swapped_tst<mode>.
            Rework signtest-and-branch peepholes for *sbrx_branch<mode>.
            (adjust_len) [add_set_ZN, cmp_zext]: New.
            (QIPSI): New mode iterator.
            (ALLs1, ALLs2, ALLs4, ALLs234): New mode iterators.
            (gelt): New code iterator.
            (gelt_eqne): New code attribute.
            (rvbranch, *rvbranch, difficult_rvbranch, *difficult_rvbranch)
            (branch_unspec, *negated_tst<mode>, *reversed_tst<mode>)
            (*cmpqi_sign_extend): Remove insns.
            (define_c_enum "unspec") [UNSPEC_IDENTITY]: Remove.
            * config/avr/avr-dimode.md (cbranch<mode>4): Canonicalize
comparisons.
            * config/avr/predicates.md (scratch_or_d_register_operand): New.
            * config/avr/constraints.md (Yxx): New constraint.

    gcc/testsuite/
            PR target/109650
            Backport from 2023-05-10 master r14-1688.
            * gcc.target/avr/torture/pr109650-1.c: New test.
            * gcc.target/avr/torture/pr109650-2.c: New test.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (11 preceding siblings ...)
  2023-06-10 20:46 ` cvs-commit at gcc dot gnu.org
@ 2023-06-10 21:08 ` gjl at gcc dot gnu.org
  2023-06-10 21:13 ` gjl at gcc dot gnu.org
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-06-10 21:08 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|saaadhu at gcc dot gnu.org         |

--- Comment #13 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
r14-1688

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Bug target/109650] avr-gcc incorrect code with -Os
  2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
                   ` (12 preceding siblings ...)
  2023-06-10 21:08 ` gjl at gcc dot gnu.org
@ 2023-06-10 21:13 ` gjl at gcc dot gnu.org
  13 siblings, 0 replies; 15+ messages in thread
From: gjl at gcc dot gnu.org @ 2023-06-10 21:13 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109650

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #14 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Fixed in v12.4+ and v13.2+.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-06-10 21:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-27 17:13 [Bug c/109650] New: avr-gcc incorrect code with -Os thierer at web dot de
2023-04-27 17:26 ` [Bug target/109650] " pinskia at gcc dot gnu.org
2023-04-28 15:09 ` gjl at gcc dot gnu.org
2023-04-28 15:13 ` gjl at gcc dot gnu.org
2023-04-28 15:14 ` [Bug other/109650] " gjl at gcc dot gnu.org
2023-04-29  9:05 ` [Bug middle-end/109650] " gjl at gcc dot gnu.org
2023-05-02 16:46 ` gjl at gcc dot gnu.org
2023-05-03 16:57 ` [Bug target/109650] " gjl at gcc dot gnu.org
2023-05-19  8:31 ` gjl at gcc dot gnu.org
2023-05-19  8:52 ` gjl at gcc dot gnu.org
2023-06-10 19:59 ` cvs-commit at gcc dot gnu.org
2023-06-10 20:25 ` cvs-commit at gcc dot gnu.org
2023-06-10 20:46 ` cvs-commit at gcc dot gnu.org
2023-06-10 21:08 ` gjl at gcc dot gnu.org
2023-06-10 21:13 ` gjl at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).