[Bug target/92729] [avr] Convert the backend to MODE_CC so it can be kept in future releases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92729 Senthil Kumar Selvaraj changed: What|Removed |Added CC||saaadhu at gcc dot gnu.org --- Comment #21 from Senthil Kumar Selvaraj --- FWIW, I was working on this as well some time in August, and had a semi working implementation going. Pip's implementation generated better code on the few benchmark workloads I compared, so I shelved my work (https://github.com/saaadhu/gcc-avr-cc0/tree/avr-cc0-squashed) I don't have the spare time now to start full fledged work on this, but I can help with any issues you run into.
[Bug target/98762] New: Wrong code for avrtiny if source is Z and destination is R30 or R31
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98762
Bug ID: 98762
Summary: Wrong code for avrtiny if source is Z and destination
is R30 or R31
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: saaadhu at gcc dot gnu.org
Target Milestone: ---
avr_out_movqi_r_mr_reg_disp_tiny skips restoration of the base pointer reg pair
(using subi/sbci) if reg_overlap_mentioned_p or reg_unused_after returns true
for the pointer reg.
In the below case, (reg:HI 30) is the base reg, and (reg:QI 31) is the
destination. reg_overlap_mentioned_p (correctly) returns true, and therefore
*both* R30 and R31 are not restored to their original values, instead of just
R31. reg_unused_after also returns true for this case, as it also checks if
reg_overlap_mentioned_p for the current insn.
This causes miscompilation if the compiler later uses R30 with the assumption
that it is unmodified (which is what movqi_insn promises). In the below case
(reduced from execute/961122-1.c), the compiler only sets R31 again (with R19),
as it believes R30 is unmodified.
Note that the bug is exposed if the splitter for HImode move to two QImode move
runs - otherwise, movhi_insn ensures both R30 and R31 are loaded again.
$ cat reduced.c
long long acc;
void addhi (short a)
{
acc += (long long) a << 32;
}
$ avr-gcc -mmcu=attiny40 -Os -dP -S reduce.c -o -
; (insn 53 52 264 (set (reg:QI 31 r31)
; (mem/c:QI (plus:HI (reg:HI 30 r30)
; (const_int 3 [0x3])) [1 acc+3 S1 A8])) "reduce.c":5:7 56
{movqi_insn}
; (expr_list:REG_EQUAL (mem/c:QI (const:HI (plus:HI (symbol_ref:HI
("acc") [flags 0x2] )
; (const_int 3 [0x3]))) [1 acc+3 S1 A8])
; (nil)))
subi r30,lo8(-(3)) ; 53 [c=4 l=3] movqi_insn/3
sbci r31,hi8(-(3))
ld r31,Z
; (insn 264 53 296 (set (mem/c:QI (plus:HI (reg/f:HI 28 r28)
; (const_int 16 [0x10])) [3 %sfp+16 S1 A8])
; (reg:QI 31 r31)) "reduce.c":5:7 56 {movqi_insn}
; (expr_list:REG_DEAD (reg:QI 31 r31)
; (nil)))
subi r28,lo8(-(16)) ; 264 [c=4 l=5] movqi_insn/2
sbci r29,hi8(-(16))
st Y,r31
subi r28,lo8((16))
sbci r29,hi8((16))
; (insn 296 264 54 (set (reg:QI 31 r31 [+1 ])
; (reg:QI 19 r19 [+1 ])) "reduce.c":5:7 56 {movqi_insn}
; (nil))
mov r31,r19 ; 296 [c=4 l=1] movqi_insn/0
; (insn 54 296 266 (set (reg:QI 31 r31)
; (mem/c:QI (plus:HI (reg:HI 30 r30)
; (const_int 4 [0x4])) [1 acc+4 S1 A8])) "reduce.c":5:7 56
{movqi_insn}
; (expr_list:REG_EQUAL (mem/c:QI (const:HI (plus:HI (symbol_ref:HI
("acc") [flags 0x2] )
; (const_int 4 [0x4]))) [1 acc+4 S1 A8])
; (nil)))
subi r30,lo8(-(4)) ; 54 [c=4 l=3] movqi_insn/3
sbci r31,hi8(-(4))
ld r31,Z
; (insn 266 54 298 (set (mem/c:QI (plus:HI (reg/f:HI 28 r28)
; (const_int 17 [0x11])) [3 %sfp+17 S1 A8])
; (reg:QI 31 r31)) "reduce.c":5:7 56 {movqi_insn}
; (expr_list:REG_DEAD (reg:QI 31 r31)
; (nil)))
subi r28,lo8(-(17)) ; 266 [c=4 l=5] movqi_insn/2
sbci r29,hi8(-(17))
st Y,r31
[Bug target/92729] [avr] Convert the backend to MODE_CC so it can be kept in future releases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92729 --- Comment #48 from Senthil Kumar Selvaraj --- Submitted https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563638.html, which addresses comments made when the work-in-progress version was submitted. There are no regression failures (save for some tests that fail because of code size increase) on atmega128, atxmega128a3, attiny40 and atmega8. Follow up patch https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563779.html improves code size regressions. More patches along the way - fixing some define_splits, and adding CC_CZN and CC_ZN modes, along with enabling the cmpelim pass.
[Bug target/92729] [avr] Convert the backend to MODE_CC so it can be kept in future releases
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92729 Senthil Kumar Selvaraj changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #49 from Senthil Kumar Selvaraj --- Both the above patches (rebased against latest master) are now committed. See https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568658.html and https://gcc.gnu.org/pipermail/gcc-patches/2021-April/569253.html. There's more work to be done to improve code gen (as described in https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563638.html), and I intend to work on those items in my spare time.
[Bug target/101188] [AVR] Miscompilation and function pointers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101188
Senthil Kumar Selvaraj changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
CC||saaadhu at gcc dot gnu.org
Ever confirmed|0 |1
Last reconfirmed||2021-06-25
--- Comment #1 from Senthil Kumar Selvaraj ---
Confirmed with 12.0.0 20210625
Here's a reduced testcase that hangs indefinitely with avrtest - the log shows
call to address 0.
$ cat fail.c
#include
#include
typedef uint8_t (*fn1)(void *a);
typedef void (*fn2)(void *a, const uint32_t *arg);
struct S {
uint8_t buffer[64];
uint16_t n;
fn2 f2;
void *a;
fn1 f1;
};
volatile uint16_t x;
void __attribute__((noinline))
foo(uint16_t n)
{
x = n;
}
void __attribute__((noinline)) testfn(struct S *self)
{
uint32_t arg;
foo(self->n);
self->n++;
self->f2(self->a, &arg);
self->buffer[0] = self->f1(self->a);
}
static unsigned char myfn2_called = 0;
static void
myfn2(void *a, const uint32_t * arg)
{
myfn2_called = 1;
}
static uint8_t
myfn1(void *a)
{ }
int main() {
struct S s;
s.n = 0; s.f2 = myfn2; s.f1 = myfn1;
testfn(&s);
if (myfn2_called != 1)
abort();
return 0;
}
$ avr-gcc -mmcu=atmega128 fail.c -O2 ~/code/avrtest/exit-atmega128.o --version
avr-gcc (GCC) 12.0.0 20210625 (experimental)
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ ~/code/avrtest/avrtest -mmcu=avr51 a.out
^C
The below code is at fault - there's an ldi to r31, followed by a load to Z
using R31:R30, and then an icall. The clobbering of r31 at 0x138 causes junk
values (0) to be read into Z, and icall calls address 0.
138: f4 e4 ldi r31, 0x44 ; 68
13a: ef 0e add r14, r31
13c: f1 1c adc r15, r1
13e: 32 96 adiwr30, 0x02 ; 2
140: 01 90 ld r0, Z+
142: f0 81 ld r31, Z
144: e0 2d mov r30, r0
146: be 01 movwr22, r28
148: 6f 5f subir22, 0xFF ; 255
14a: 7f 4f sbcir23, 0xFF ; 255
14c: d7 01 movwr26, r14
14e: 8d 91 ld r24, X+
150: 9c 91 ld r25, X
152: 09 95 icall
