Bug 41894 - wrong code with -fno-split-wide-types
Summary: wrong code with -fno-split-wide-types
Status: RESOLVED DUPLICATE of bug 46779
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.3.2
: P3 normal
Target Milestone: 4.6.2
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2009-10-31 21:43 UTC by Frank Boesing
Modified: 2011-07-23 15:13 UTC (History)
4 users (show)

See Also:
Host:
Target: avr-*-*
Build:
Known to work:
Known to fail: 4.3.2, 4.5.0, 4.5.2, 4.6.1
Last reconfirmed: 2009-10-31 23:55:40


Attachments
Full assembler code I get from GCC 4.3.2 (413 bytes, application/octet-stream)
2009-10-31 23:10 UTC, Joerg Wunsch
Details
dump file (3.08 KB, application/octet-stream)
2009-11-01 17:27 UTC, Andy Hutchinson
Details
dump file (3.33 KB, application/octet-stream)
2009-11-01 17:27 UTC, Andy Hutchinson
Details
simple test case (196 bytes, text/plain)
2011-05-21 14:47 UTC, Ilya Lesokhin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Boesing 2009-10-31 21:43:57 UTC
Commandline:
avr-gcc -S -mmcu=atmega128 -Os 
-fno-inline-small-functions -fno-split-wide-types bug.c

Sourcecode:

typedef unsigned char uint8_t;
typedef unsigned short uint16_t;

typedef union
{
  struct {
    uint8_t sekunden;
    uint8_t minuten;
  } x;
  uint16_t sekmin;
} zeit_t;


void testmich2 (zeit_t tmp) {
  // just something that cannot be optimized out
  asm volatile("nop");
}

void testmich (zeit_t zeit) {

  zeit_t tmp;

  tmp.x = zeit.x;

  do {
    testmich2(tmp);

    if (tmp.x.sekunden)
      tmp.x.sekunden--;
    else {
      tmp.x.sekunden = 59;
      tmp.x.minuten--;
    }
  } while (tmp.x.sekunden || tmp.x.minuten);
}

int main (void) {
  zeit_t zeit;

  zeit.x.minuten = 1;
  zeit.x.sekunden = 2;

  testmich(zeit);

  return 0;
}


The statements

if (tmp.x.sekunden)
      tmp.x.sekunden--;

translate to:

        mov r18,r28
        subi r18,lo8(-(-1))
        movw r28,r18

which is wrong; r19 (used by movw) has a not defined value.
Comment 1 Andy Hutchinson 2009-10-31 23:02:37 UTC
Please post entire assembler code.
Comment 2 Joerg Wunsch 2009-10-31 23:10:27 UTC
Created attachment 18944 [details]
Full assembler code I get from GCC 4.3.2
Comment 3 Joerg Wunsch 2009-10-31 23:11:51 UTC
The bug was originally reported in the (Germany-language) mikrocontroller.net
forum, and I confirmed the bug on my local GCC 4.3.2 setup before asking
Frank to submit it as an official bug report.
Comment 4 Andy Hutchinson 2009-11-01 17:24:35 UTC
The problem is still present on 4.3.5 head
I cannot reproduce on 4.5 

It looks like reload issue with SUBREG. 

Instruction 18 gets reloaded. The output reload is HImode. I will post dump files but here is extract that appears to highlight problem.

From lreg dump file:

;; Pred edge  3 [50.0%]  (fallthru)
(note 16 15 17 4 [bb 4] NOTE_INSN_BASIC_BLOCK)

(note 17 16 18 4 NOTE_INSN_DELETED)

(insn 18 17 54 4 pr41894.c:29 (set (subreg:QI (reg/v:HI 43 [ tmp ]) 0)
        (plus:QI (reg:QI 42 [ D.1188 ])
            (const_int -1 [0xffffffff]))) 15 {addqi3} (expr_list:REG_DEAD (reg:QI 42 [ D.1188 ])
        (nil)))

(jump_insn 54 18 55 4 (set (pc)
        (label_ref 26)) 101 {jump} (nil))
;; End of basic block 4 -> ( 6)
;; lr  out 	 28 [r28] 32 [__SP_L__] 34 [argL] 43
;; live  out 	 28 [r28] 32 [__SP_L__] 34 [argL] 43


from greg dump file:

Spilling for insn 18.
Using reg 18 for reload 0
Spilling for insn 18.
Using reg 18 for reload 0

Reloads for insn # 18
Reload 0: reload_in (QI) = (reg:QI 24 r24 [orig:42 D.1188 ] [42])
	reload_out (HI) = (reg/v:HI 28 r28 [orig:43 tmp ] [43])
	LD_REGS, RELOAD_OTHER (opnum = 0)
	reload_in_reg: (reg:QI 24 r24 [orig:42 D.1188 ] [42])
	reload_out_reg: (reg/v:HI 28 r28 [orig:43 tmp ] [43])
	reload_reg_rtx: (reg:HI 18 r18)
deleting insn with uid = 2.
;; Register dispositions:
42 in 24  43 in 28  44 in 24  

;; Hard regs used:  18 19 24 25 28 29 32


Giving


(insn 57 17 18 4 pr41894.c:29 (set (reg:QI 18 r18)
        (reg:QI 24 r24 [orig:42 D.1188 ] [42])) 4 {*movqi} (nil))

(insn 18 57 58 4 pr41894.c:29 (set (reg:QI 18 r18)
        (plus:QI (reg:QI 18 r18)
            (const_int -1 [0xffffffff]))) 15 {addqi3} (nil))

(insn 58 18 54 4 pr41894.c:29 (set (reg/v:HI 28 r28 [orig:43 tmp ] [43])
        (reg:HI 18 r18)) 8 {*movhi} (nil))

(jump_insn 54 58 55 4 (set (pc)
Comment 5 Andy Hutchinson 2009-11-01 17:27:32 UTC
Created attachment 18945 [details]
dump file
Comment 6 Andy Hutchinson 2009-11-01 17:27:55 UTC
Created attachment 18946 [details]
dump file
Comment 7 Eric Weddington 2009-11-01 17:44:51 UTC
(In reply to comment #4)
> The problem is still present on 4.3.5 head
> I cannot reproduce on 4.5 

Can someone check this to see if bug exists on any 4.4.x?
Comment 8 Andy Hutchinson 2009-11-02 00:54:02 UTC
The problem seems related to use of R28+r29 - which is also frame pointer.

avr_hard_regno_mode_ok allows R28 in HIMODE but not any other mode. (This hack was made to avoid reload problem where r29 was used as well as R28 frame pointer)

It looks like the "not ok" QI subreg 28/29 of "ok" HImode r28 is not handled well. 

The mode restriction is ignored for input reload. (Perhaps because reload decides to use available frame pointer)

The output reload needed is 16bit HImode but must be accessible as two QImode  subregs. Somehow the mode of output reload gets mangled so we get movw r28,r18


Originally ~2005 the rejection of other modes by avr_hard_regno_mode_ok for r28 was only applied when reload was in progress and frame_pointer_required.

So I reapplied this condition and the bug was solved. This function does not need frame pointer so r28 and r29 are indeed freely available.

  /* Otherwise disallow all regno/mode combinations that span r28:r29.  */
  if (reload_in_progress && frame_pointer_needed && regno <= (REG_Y + 1) && (regno + GET_MODE_SIZE (mode)) >= (REG_Y + 1))
    return 0;


This also produces better code!

One concern is that the modes deemed ok or not are now not constant - and perhaps is the reason why condition was removed.

In Gcc 4.5 the spill is exactly the same but it uses R16/r17 with the correct output reload subreg - so seems to figure out that r28 is "not ok" - suggesting problem is fixed.


This may take a while to resolve, with some further research and testing.




Comment 9 Eric Weddington 2010-09-20 03:03:40 UTC
Closing as fixed in 4.5.0.
Comment 10 Georg-Johann Lay 2011-03-13 12:07:06 UTC
(In reply to comment #9)
> Closing as fixed in 4.5.0.

Why and how is this bug fixed? Slight variations in source/compiler will make this bug appear/disappear, so that testing the same source against other compiler version does not tell wether or not the bug actually has gone.

QImode still is not allowed in r28/r29.

PR46779 (4.4.x) and PR45291 (4.5.x) are duplicates for this bug in different compiler versions.

Some passes like subreg lowering, RTL lowering generate 
  (set (subreg:QI (REG:HI ...
where HI reg finally gets allocated to r29:r28, however, the subreg is not allowed in r28 resp. r29 leading to wrong code.

See also comment in 
http://gcc.gnu.org/ml/gcc/2011-02/msg00498.html

All the hacks in avr_hard_regno_mode_ok just work around prolems somewhere else, and maybe around problems that have been fixed long ago.

IMO avr_hard_regno_mode_ok should be written according to documentation and look what regressions that triggers, if any.

Johann
Comment 11 Georg-Johann Lay 2011-04-14 18:50:15 UTC
Author: gjl
Date: Thu Apr 14 18:50:02 2011
New Revision: 172442

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172442
Log:
	PR target/46779
	PR target/45291
	PR target/41894
	* gcc.target/avr/pr46779-1.c: New test case
	* gcc.target/avr/pr46779-2.c: New test case


Added:
    trunk/gcc/testsuite/gcc.target/avr/pr46779-1.c
    trunk/gcc/testsuite/gcc.target/avr/pr46779-2.c
Modified:
    trunk/gcc/testsuite/ChangeLog
Comment 12 Ilya Lesokhin 2011-05-21 14:47:33 UTC
Created attachment 24319 [details]
simple test case

i belive this is the same bug.

the problem occurs when using r28,r29 and accesing one of them.

as can be seen badCode() doesnt put the value 5 in tester. while goodCode does.


void badCode()
{
  a4:	ef 92       	push	r14
  a6:	ff 92       	push	r15
  a8:	cf 93       	push	r28
  aa:	df 93       	push	r29
	} t;

	t.deletedbyte = 5;
	asm (""	: "+y" (t) :);

	t.otherbyte = 17;
  ac:	81 e1       	ldi	r24, 0x11	; 17
  ae:	ec 01       	movw	r28, r24

	asm (""	: "+y" (t) :);

	tester = t.deletedbyte;
  b0:	7e 01       	movw	r14, r28
  b2:	f0 92 00 01 	sts	0x0100, r15
}
  b6:	df 91       	pop	r29
  b8:	cf 91       	pop	r28
  ba:	ff 90       	pop	r15
  bc:	ef 90       	pop	r14
  be:	08 95       	ret

000000c0 <goodCode>:

void goodCode()
{
  c0:	a0 e0       	ldi	r26, 0x00	; 0
  c2:	b5 e0       	ldi	r27, 0x05	; 5
	} t;

	t.deletedbyte = 5;
	asm (""	: "+x" (t) :);

	t.otherbyte = 17;
  c4:	a1 e1       	ldi	r26, 0x11	; 17

	asm (""	: "+x" (t) :);

	tester = t.deletedbyte;
  c6:	b0 93 00 01 	sts	0x0100, r27
}
  ca:	08 95       	ret
Comment 13 Georg-Johann Lay 2011-07-23 15:09:49 UTC

*** This bug has been marked as a duplicate of bug 46779 ***
Comment 14 Georg-Johann Lay 2011-07-23 15:13:45 UTC
Adapted known-o-fail as of attachement 24319 from Ilya.