Bug 35729 - const volatile variable access incorrectly hoisted out of loop
Summary: const volatile variable access incorrectly hoisted out of loop
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: 4.4.0
Assignee: Zdenek Dvorak
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2008-03-28 03:04 UTC by John Regehr
Modified: 2009-11-29 16:38 UTC (History)
8 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work: 4.1.3 4.4.0
Known to fail: 4.2.3 4.3.0
Last reconfirmed: 2009-01-17 02:50:47


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John Regehr 2008-03-28 03:04:49 UTC
This is for "gcc version 4.3.0 (GCC)" 

func_1() here should never load from g_361 since the loop body never executes:

  const volatile int g_361 = 3L;
  volatile int g_2 = 0L;
  void func_1 (void) {
    for (g_2 = 0; g_2 > 10; g_2++)
      {
        int l_357 = g_361;
      }
  }

However when compiled with "gcc -Os -S foo.c" we get the following output which does load from g_361:

	.file	"foo.c"
	.text
.globl func_1
	.type	func_1, @function
func_1:
	pushl	%ebp
	movl	%esp, %ebp
	movl	$0, g_2
	movl	g_361, %eax
	jmp	.L2
.L3:
	movl	g_2, %eax
	incl	%eax
	movl	%eax, g_2
.L2:
	movl	g_2, %eax
	cmpl	$10, %eax
	jg	.L3
	popl	%ebp
	ret
	.size	func_1, .-func_1
.globl g_361
	.data
	.align 4
	.type	g_361, @object
	.size	g_361, 4
g_361:
	.long	3
.globl g_2
	.bss
	.align 4
	.type	g_2, @object
	.size	g_2, 4
g_2:
	.zero	4
	.ident	"GCC: (GNU) 4.3.0"
	.section	.note.GNU-stack,"",@progbits

At -O0, -O1, and -O2 there is no problem.
Comment 1 Richard Biener 2008-03-28 10:26:31 UTC
Confirmed.  RTL loop invariant motion moves the volatile load out of the
function.
Comment 2 Zdenek Dvorak 2008-03-31 14:20:50 UTC
Subject: Bug 35729

Author: rakdver
Date: Mon Mar 31 14:19:52 2008
New Revision: 133755

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133755
Log:
	PR rtl-optimization/35729
	* loop-invariant.c (check_maybe_invariant): Disallow volatile memory
	references.

	* gcc.dg/pr35729.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/pr35729.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/loop-invariant.c
    trunk/gcc/testsuite/ChangeLog

Comment 3 Dominique d'Humieres 2008-04-01 09:39:30 UTC
The test fails on i686-apple-darwin9 at revision 133785:

FAIL: gcc.dg/pr35729.c scan-rtl-dump-times loop2_invariant "Decided to move invariant" 0

Comment 4 Dominique d'Humieres 2008-04-01 10:28:15 UTC
On i686-apple-darwin9, the failure occurs only in 32 bit mode (default). I also occurs on powerpc-apple-darwin8.5.0:

http://gcc.gnu.org/ml/gcc-testresults/2008-04/msg00013.html


Comment 5 John David Anglin 2008-04-27 17:17:24 UTC
The test pr35729.c also fails on hppa-unknown-linux-gnu.
Comment 6 Kaveh Ghazi 2008-05-25 18:03:19 UTC
The testcase also fails for me on x86_64-unknown-linux-gnu or i686-unknown-linux-gnu but requires -fpic/-fPIC to trigger.  (That may explain the darwin x86 error.)  See:

x86_64: http://gcc.gnu.org/ml/gcc-testresults/2008-05/msg02221.html
i686: http://gcc.gnu.org/ml/gcc-testresults/2008-05/msg01800.html

Comment 7 Steve Ellcey 2008-10-16 21:32:53 UTC
The new test that was added fails for me on ia64-*-* platforms too.  It looks like the fix for the original bug is right in that it is preventing the volatile assignment in being moved but the test is bad because other instructions (not involving the volatile variable) are being moved out of the loop so the test does see the "Decided to move invariant" string in the loop dump file.  The information in this file doesn't seem to be specific enough to allow for an easy way to check whether the assignment of g_361 specifically was or was not moved.  If I compile with -O2 instead of -Os then I don't see any invariant code motion but I don't know if that is a way to fix the test or not since the original bug involved -Os.
Comment 8 John David Anglin 2008-11-08 01:34:50 UTC
The test pr35729.c also fails on hppa64-hp-hpux11.11.
Comment 9 Eric Botcazou 2008-11-08 20:19:15 UTC
> The test pr35729.c also fails on hppa64-hp-hpux11.11.

Same on the SPARC.
Comment 10 Andreas Krebbel 2008-12-02 14:19:01 UTC
Fails on s390 and s390x as well.
Comment 11 Steve Ellcey 2009-01-05 23:15:40 UTC
Zdenek, are you still looking at this bug?  As I mention in comment #7, I think the fix that is checked in is good, it is just the test that is bad.  I don't see a good way to fix the test, I would support just removing the test, though I can't actually approve such a change.  Do you see a better way to address the problem of the failing test?
Comment 12 Kaveh Ghazi 2009-01-17 02:50:47 UTC
Reconfirming for (x86 && pic):

http://gcc.gnu.org/ml/gcc-testresults/2009-01/msg01601.html
Comment 13 Uroš Bizjak 2009-01-19 18:53:42 UTC
(In reply to comment #10)
> Fails on s390 and s390x as well.

And alpha.

Comment 14 Kaveh Ghazi 2009-11-29 16:21:53 UTC
This testcase was "fixed" here:
http://gcc.gnu.org/ml/gcc-patches/2009-01/msg01134.html

Can we close this one?

Comment 15 Richard Biener 2009-11-29 16:38:14 UTC
Fixed.