Bug 21171 - [4.0/4.1 Regression] IV OPTS removes does not create a new VOPs for constant values
Summary: [4.0/4.1 Regression] IV OPTS removes does not create a new VOPs for constant ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.0.0
: P1 critical
Target Milestone: 4.0.1
Assignee: Zdenek Dvorak
URL:
Keywords: alias, wrong-code
Depends on:
Blocks:
 
Reported: 2005-04-23 08:18 UTC by Sami Kantoluoto
Modified: 2005-06-13 19:43 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-04-26 18:32:33


Attachments
A bit more simplified test file with a test code that gets compiled correctly commented out. (360 bytes, text/plain)
2005-04-23 08:31 UTC, Sami Kantoluoto
Details
objdump output of working version when compiled in thumb mode (-mthumb) (543 bytes, text/plain)
2005-04-23 08:45 UTC, Sami Kantoluoto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sami Kantoluoto 2005-04-23 08:18:15 UTC
Compiling the test code with optimizations enabled generates invalid code. Data
gets read from the source table but gets not written to the destination.

Test code:
/* start of test.c */

/* compile: arm-elf-gcc -Os -c -o test.o test.c */
/*    dump: arm-elf-objdump -D test.o | more    */

typedef unsigned int u_int32_t;
typedef unsigned char u_int8_t;

#define AIC_VECTORS     32
        
typedef volatile struct AT91RM9200_AIC {
  u_int32_t             SVR[AIC_VECTORS];
} AT91RM9200_AIC_t;
         
typedef volatile struct AT91RM9200_regs {
  AT91RM9200_AIC_t      AIC;
} AT91RM9200_regs_t;
            
#define CPUReg          ((AT91RM9200_regs_t*)0xFFF00000)
        
extern const u_int32_t __IntTable[AIC_VECTORS];
        
int main()
{
  int c;
  AT91RM9200_AIC_t *aic = &CPUReg->AIC;
            
  for (c=0; c < AIC_VECTORS; c++) {
    aic->SVR[c] = __IntTable[c];
  }

  return 0;
}
/* end of test.c */

objdump output:

test.o:     file format elf32-littlearm
        
Disassembly of section .text:
        
00000000 <main>:
   0:   e59f2014        ldr     r2, [pc, #20]   ; 1c <.text+0x1c>
   4:   e59f3014        ldr     r3, [pc, #20]   ; 20 <.text+0x20>
   8:   e2822004        add     r2, r2, #4      ; 0x4
   c:   e1520003        cmp     r2, r3
  10:   1affffff        bne     4 <main+0x4>
  14:   e3a00000        mov     r0, #0  ; 0x0
  18:   e12fff1e        bx      lr

As you can see, there is no store (str) instruction at all. Code just goes
through the __IntTable[] but does nothing useful.
Comment 1 Sami Kantoluoto 2005-04-23 08:31:09 UTC
Created attachment 8713 [details]
A bit more simplified test file with a test code that gets compiled correctly commented out.

Code that gets compiled _corretly_:
    *(&CPUReg->SVR[c]) = __IntTable[c];

And code that doesn't:
    CPUReg->SVR[c] = __IntTable[c];


Objdump of working code:

test.o:     file format elf32-littlearm

Disassembly of section .text:

00000000 <main>:
   0:	e59f101c	ldr	r1, [pc, #28]	; 24 <.text+0x24>
   4:	e59f001c	ldr	r0, [pc, #28]	; 28 <.text+0x28>
   8:	e4912004	ldr	r2, [r1], #4
   c:	e59f3018	ldr	r3, [pc, #24]	; 2c <.text+0x2c>
  10:	e1510003	cmp	r1, r3
  14:	e4802004	str	r2, [r0], #4
  18:	1a000000	bne	8 <main+0x8>
  1c:	e3a00000	mov	r0, #0	; 0x0
  20:	e12fff1e	bx	lr

(this could be optimized by not reloading the r3 every loop).
Comment 2 Sami Kantoluoto 2005-04-23 08:45:26 UTC
Created attachment 8714 [details]
objdump output of working version when compiled in thumb mode (-mthumb)

Same problems in thumb mode. Attachment shows the objdump output of the
_working_ version. Objdump output of non-working version is here:

test.o:     file format elf32-littlearm

Disassembly of section .text:

00000000 <main>:
   0:	4b03		ldr	r3, [pc, #12]	(10 <.text+0x10>)
   2:	1c1a		mov	r2, r3		(add r2, r3, #0)
   4:	3280		add	r2, #128
   6:	3304		add	r3, #4
   8:	4293		cmp	r3, r2
   a:	d1fc		bne	6 <main+0x6>
   c:	2000		mov	r0, #0
   e:	4770		bx	lr
  10:	0000		lsl	r0, r0, #0
	...
Disassembly of section .comment:

00000000 <.comment>:
   0:	43434700	cmpmi	r3, #0	; 0x0
   4:	4728203a	undefined
   8:	2029554e	eorcs	r5, r9, lr, asr #10
   c:	2e302e34	mrccs	14, 1, r2, cr0, cr4, {1}

As you can see, it does not even access the source table (non-thumb version
does).
Comment 3 Richard Earnshaw 2005-04-26 18:32:31 UTC
Confirmed.  I suspect this is a tree loop optimization bug.  The final tree dump
has:

main ()
{
  void * D.1236;
  void * ivtmp.8;

<bb 0>:
  ivtmp.8 = &__IntTable[0];

<L0>:;
  ivtmp.8 = ivtmp.8 + 4B;
  D.1236 = &__IntTable[0] + 128B;
  if (ivtmp.8 != D.1236) goto <L0>; else goto <L2>;

<L2>:;
  return 0;

}
Comment 4 Andrew Pinski 2005-04-26 19:51:22 UTC
The problem comes from dce but the problem is that IV OPT is making the pointer as "unsigned int *", 
because the field's type is just "unsigned int[]" but since the orginal type of the struct is volatile, we 
don't have a vop as it is volatile store. And since IV opts just copies the vops and not run aliasing (which 
right now causes a different issue on 4.0 branch), we don't get a vop.
Comment 5 Andrew Pinski 2005-04-26 22:17:59 UTC
Removing volatile also causes the same problem,  Ok my analysis in comment #4 is not fully current, 
we still did not have a vop to begin with but since ivopts changes it to be a slightly different loop, a 
pointer based one, we need a vop as the access is no longer to a deferencing a (constant + variable).
Comment 6 GCC Commits 2005-04-27 14:28:26 UTC
Subject: Bug 21171

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	rakdver@gcc.gnu.org	2005-04-27 14:28:12

Modified files:
	gcc            : ChangeLog tree-ssa-loop-ivopts.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.dg/tree-ssa: pr21171.c 

Log message:
	PR tree-optimization/21171
	* tree-ssa-loop-ivopts.c (find_interesting_uses_address): Do not
	record address uses if the reference is volatile.
	
	* gcc.dg/tree-ssa/pr21171.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.8479&r2=2.8480
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-ssa-loop-ivopts.c.diff?cvsroot=gcc&r1=2.63&r2=2.64
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.5404&r2=1.5405
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/tree-ssa/pr21171.c.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 7 Mark Mitchell 2005-05-26 22:13:38 UTC
Is it possible to backport this patch to 4.0.x?
Comment 8 Andrew Pinski 2005-06-02 14:35:46 UTC
The patch did not fix the orginal testcase because the problem is with constant values.
Comment 9 GCC Commits 2005-06-13 19:24:43 UTC
Subject: Bug 21171

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-4_0-branch
Changes by:	rakdver@gcc.gnu.org	2005-06-13 19:24:33

Modified files:
	gcc            : ChangeLog tree-ssa-loop-ivopts.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.dg/tree-ssa: pr21171.c 

Log message:
	PR tree-optimization/21171
	* tree-ssa-loop-ivopts.c (find_interesting_uses_address): Do not
	record address uses if the reference is volatile.
	
	* gcc.dg/tree-ssa/pr21171.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.7592.2.285&r2=2.7592.2.286
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-ssa-loop-ivopts.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.49.2.3&r2=2.49.2.4
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.5084.2.236&r2=1.5084.2.237
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/tree-ssa/pr21171.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.6.1

Comment 10 Andrew Pinski 2005-06-13 19:43:12 UTC
Fixed.