Bug 11381 - [3.3/3.4 regression] volatile memory access optimized away
Summary: [3.3/3.4 regression] volatile memory access optimized away
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 3.3
: P1 critical
Target Milestone: 3.3.1
Assignee: Eric Botcazou
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2003-06-30 16:25 UTC by Tobias Ringström
Modified: 2004-01-17 04:22 UTC (History)
1 user (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2003-07-01 17:11:17


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Ringström 2003-06-30 16:25:21 UTC
Example code:

void
flash_wait_ready(volatile unsigned *vaddr)
{
    while (*vaddr != *vaddr)
        ;
}

Using gcc 3.3 (tested with arm-elf and i686-pc-linux-gnu), the loop in the
example above is incorrectly removed by the optimizer when using -O. Correct
code is generated using -O2, -O3 and -Os. Here is the -O case:

> gcc -S -o - -O tmp.c
    .file    "tmp.c"
    .text
.globl flash_wait_ready
    .type    flash_wait_ready, @function
flash_wait_ready:
    pushl    %ebp
    movl    %esp, %ebp
    popl    %ebp
    ret
    .size    flash_wait_ready, .-flash_wait_ready
    .ident    "GCC: (GNU) 3.3"
>

It's a regression from GCC 3.2.3 which generates correct code with -O.

[I know the code looks funny, but as the function name indicates I'm waiting for
a flash memory to finish. When the flash is busy, two consecutive read accesses
will not read the same value.]

/Tobias
Comment 1 Tobias Ringström 2003-06-30 16:26:13 UTC
Info from Andrew Pinski:

Please file a bug report but I still see it on the mainline ().
Also Here is a workaround:
void flash_wait_ready(volatile unsigned *vaddr)
{
        while(1)
        {
                unsigned temp = *vaddr;
                if (temp != *vaddr)
                        break;
        }
}

Thanks,
Andrew Pinski 
Comment 2 Wolfgang Bangerth 2003-06-30 16:36:04 UTC
Confirmed, a regression in 3.3 and mainline w.r.t. 3.2. Here's another 
work-around:
---------------------------
void
flash_wait_ready(unsigned * volatile vaddr)
{
    while (*vaddr != *vaddr)
        ;
}
---------------------------

I must admit that I think that this one is wrong, though, as it
makes the _pointer_ volatile, not the object pointed to.

W.
Comment 3 Eric Botcazou 2003-07-01 17:10:49 UTC
Fixing.
Comment 4 GCC Commits 2003-07-03 07:30:11 UTC
Subject: Bug 11381

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	ebotcazou@gcc.gnu.org	2003-07-03 07:30:03

Modified files:
	gcc            : ChangeLog simplify-rtx.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.dg: i386-volatile-1.c 

Log message:
	PR optimization/11381
	* simplify-rtx.c (simplify_relational_operation): Check that
	two equal operands have no side-effects before simplifying
	the comparison.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.308&r2=2.309
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/simplify-rtx.c.diff?cvsroot=gcc&r1=1.144&r2=1.145
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.2829&r2=1.2830
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/i386-volatile-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 5 GCC Commits 2003-07-03 07:38:32 UTC
Subject: Bug 11381

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	ebotcazou@gcc.gnu.org	2003-07-03 07:38:29

Modified files:
	gcc            : ChangeLog simplify-rtx.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.dg: i386-volatile-1.c 

Log message:
	PR optimization/11381
	* simplify-rtx.c (simplify_relational_operation): Check that
	two equal operands have no side-effects before simplifying
	the comparison.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.16114.2.631&r2=1.16114.2.632
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/simplify-rtx.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.126.4.2&r2=1.126.4.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.2261.2.214&r2=1.2261.2.215
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/i386-volatile-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=NONE&r2=1.1.2.1

Comment 6 Eric Botcazou 2003-07-03 07:43:39 UTC
See http://gcc.gnu.org/ml/gcc-patches/2003-07/msg00175.html
Comment 7 Tobias Ringström 2003-07-03 08:09:00 UTC
I've tested the fix on arm-elf by applying your patch to simplify-rtx.c to the
gcc-3.3 source. I then compiled my original code and it works perfectly. Thanks
a lot for the quick response. I'm impressed!