Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1278651 > unrolled thread

no-op delay loops

Back to article view | Back to linux.kernel

Contents

  no-op delay loops Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-11-27 10:00 +0100
    Re: no-op delay loops yalin wang <yalin.wang2010@gmail.com> - 2015-11-27 10:10 +0100
      Re: no-op delay loops Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-27 10:30 +0100
    Re: no-op delay loops Arnd Bergmann <arnd@arndb.de> - 2015-11-27 12:20 +0100
      Re: no-op delay loops Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-11-30 22:30 +0100
        Re: no-op delay loops Arnd Bergmann <arnd@arndb.de> - 2015-11-30 22:50 +0100
        Re: no-op delay loops Ralf Baechle <ralf@linux-mips.org> - 2015-12-01 02:30 +0100
        Re: no-op delay loops Richard Henderson <rth@twiddle.net> - 2015-12-01 16:40 +0100
    Re: no-op delay loops Ralf Baechle <ralf@linux-mips.org> - 2015-11-27 12:50 +0100

#1278651 — no-op delay loops

Hi,

It seems that gcc happily compiles

for (i = 0; i < 1000000000; ++i) ;

into simply

i = 1000000000;

(which is then usually eliminated as a dead store). At least at -O2, and
when i is not declared volatile. So it would seem that the loops at

arch/mips/pci/pci-rt2880.c:235
arch/mips/pmcs-msp71xx/msp_setup.c:80
arch/mips/sni/reset.c:35

actually don't do anything. (In the middle one, i is 'register', but
that doesn't change anything.) Is mips compiled with some special flags
that would make gcc actually emit code for the above?

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1278659

> On Nov 27, 2015, at 16:53, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> 
> Hi,
> 
> It seems that gcc happily compiles
> 
> for (i = 0; i < 1000000000; ++i) ;
> 
> into simply
> 
> i = 1000000000;
> 
> (which is then usually eliminated as a dead store). At least at -O2, and
> when i is not declared volatile. So it would seem that the loops at
> 
> arch/mips/pci/pci-rt2880.c:235
> arch/mips/pmcs-msp71xx/msp_setup.c:80
> arch/mips/sni/reset.c:35
> 
> actually don't do anything. (In the middle one, i is 'register', but
> that doesn't change anything.) Is mips compiled with some special flags
> that would make gcc actually emit code for the above?
> 
you can try to declare i as  volatile int i;
may gcc will not optimize it .

Thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278669

On Fri, Nov 27, 2015 at 11:04 AM, yalin wang <yalin.wang2010@gmail.com> wrote:
>> On Nov 27, 2015, at 16:53, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

>> It seems that gcc happily compiles
>>
>> for (i = 0; i < 1000000000; ++i) ;
>>
>> into simply
>>
>> i = 1000000000;
>>
>> (which is then usually eliminated as a dead store). At least at -O2, and
>> when i is not declared volatile. So it would seem that the loops at
>>
>> arch/mips/pci/pci-rt2880.c:235
>> arch/mips/pmcs-msp71xx/msp_setup.c:80
>> arch/mips/sni/reset.c:35
>>
>> actually don't do anything. (In the middle one, i is 'register', but
>> that doesn't change anything.) Is mips compiled with some special flags
>> that would make gcc actually emit code for the above?
>>
> you can try to declare i as  volatile int i;
> may gcc will not optimize it .

Might be, but Rasmus as I can see asked about *existing* code.


-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278756

On Friday 27 November 2015 09:53:50 Rasmus Villemoes wrote:
> 
> It seems that gcc happily compiles
> 
> for (i = 0; i < 1000000000; ++i) ;
> 
> into simply
> 
> i = 1000000000;
> 
> (which is then usually eliminated as a dead store). At least at -O2, and
> when i is not declared volatile. So it would seem that the loops at
> 
> arch/mips/pci/pci-rt2880.c:235
> arch/mips/pmcs-msp71xx/msp_setup.c:80
> arch/mips/sni/reset.c:35
> 
> actually don't do anything. (In the middle one, i is 'register', but
> that doesn't change anything.) Is mips compiled with some special flags
> that would make gcc actually emit code for the above?
> 

I remember that gcc used to not optimize code that looked like a
delay loop such as the above, and my tests show that this was still
the case in gcc-4.0.3, but starting with gcc-4.1 it opimtized away
that loop.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280298

On Fri, Nov 27 2015, Arnd Bergmann <arnd@arndb.de> wrote:

> On Friday 27 November 2015 09:53:50 Rasmus Villemoes wrote:
>> 
>> It seems that gcc happily compiles
>> 
>> for (i = 0; i < 1000000000; ++i) ;
>> 
>> into simply
>> 
>> i = 1000000000;
>> 
>> (which is then usually eliminated as a dead store). At least at -O2, and
>> when i is not declared volatile. So it would seem that the loops at
>> 
>> arch/mips/pci/pci-rt2880.c:235
>> arch/mips/pmcs-msp71xx/msp_setup.c:80
>> arch/mips/sni/reset.c:35
>> 
>> actually don't do anything. (In the middle one, i is 'register', but
>> that doesn't change anything.) Is mips compiled with some special flags
>> that would make gcc actually emit code for the above?
>> 
>
> I remember that gcc used to not optimize code that looked like a
> delay loop such as the above, and my tests show that this was still
> the case in gcc-4.0.3, but starting with gcc-4.1 it opimtized away
> that loop.

OK, thanks. That's a very very long time ago.

FWIW, the remaining instances that my trivial coccinelle script found
are

./arch/alpha/boot/main.c:187:1-4: no-op delay loop
./arch/m68k/68000/m68VZ328.c:86:10-13: no-op delay loop
./arch/m68k/bvme6000/config.c:338:2-5: no-op delay loop
./arch/m68k/coldfire/m53xx.c:533:1-4: no-op delay loop
./drivers/cpufreq/cris-artpec3-cpufreq.c:85:3-6: no-op delay loop
./drivers/cpufreq/cris-etraxfs-cpufreq.c:85:3-6: no-op delay loop
./drivers/tty/hvc/hvc_opal.c:313:3-6: no-op delay loop
./drivers/tty/hvc/hvc_vio.c:289:3-6: no-op delay loop

(cc += a few people). The tty ones use volatile, so they probably work,
though one might still want to use the *delay API.

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280318

On Monday 30 November 2015 22:29:26 Rasmus Villemoes wrote:
> 
> OK, thanks. That's a very very long time ago.
> 
> FWIW, the remaining instances that my trivial coccinelle script found
> are
> 
> ./arch/alpha/boot/main.c:187:1-4: no-op delay loop
> ./arch/m68k/68000/m68VZ328.c:86:10-13: no-op delay loop
> ./arch/m68k/bvme6000/config.c:338:2-5: no-op delay loop
> ./arch/m68k/coldfire/m53xx.c:533:1-4: no-op delay loop
> ./drivers/cpufreq/cris-artpec3-cpufreq.c:85:3-6: no-op delay loop
> ./drivers/cpufreq/cris-etraxfs-cpufreq.c:85:3-6: no-op delay loop
> ./drivers/tty/hvc/hvc_opal.c:313:3-6: no-op delay loop
> ./drivers/tty/hvc/hvc_vio.c:289:3-6: no-op delay loop
> 
> (cc += a few people). The tty ones use volatile, so they probably work,
> though one might still want to use the *delay API.
> 
> 

I suspect the tty users do it like this to get console output
before calibrate_delay_loop() is called.

	Ard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280467

On Mon, Nov 30, 2015 at 10:29:26PM +0100, Rasmus Villemoes wrote:

> OK, thanks. That's a very very long time ago.
> 
> FWIW, the remaining instances that my trivial coccinelle script found
> are

After your initial report I also wrote a coccinelle which is looking
also for delay loops implemented in while loops.  It found the following
two:

diff -u -p ./drivers/video/uvesafb.c /tmp/nothing/drivers/video/uvesafb.c
--- ./drivers/video/uvesafb.c
+++ /tmp/nothing/drivers/video/uvesafb.c
@@ -1142,7 +1142,6 @@ static int uvesafb_blank(int blank, stru
 		vga_wseq(NULL, 0x00, seq);
 
 		crtc17 |= vga_rcrt(NULL, 0x17) & ~0x80;
-		while (loop--);
 		vga_wcrt(NULL, 0x17, crtc17);
 		vga_wseq(NULL, 0x00, 0x03);
 	} else
diff -u -p ./arch/mips/pmc-sierra/yosemite/atmel_read_eeprom.c /tmp/nothing/arch/mips/pmc-sierra/yosemite/atmel_read_eeprom.c
--- ./arch/mips/pmc-sierra/yosemite/atmel_read_eeprom.c
+++ /tmp/nothing/arch/mips/pmc-sierra/yosemite/atmel_read_eeprom.c
@@ -37,7 +37,6 @@
 
 static void delay(int delay)
 {
-	while (delay--);
 }
 
 static void send_bit(unsigned char bit)

The 2nd file falls into my domain so I'm going to fix it.  Not sure
how the uvesafb one should be treated.

  Ralf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280972

On 11/30/2015 01:29 PM, Rasmus Villemoes wrote:
> ./arch/alpha/boot/main.c:187:1-4: no-op delay loop

Not sure why this is there.  The runkernel function that preceeds it cannot 
return, having performed an indirect branch (not a call) to the kernel start.

I see no reason to actually change anything, unless someone insists that this 
be remove to avoid future similar analysis.


r~
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278768

On Fri, Nov 27, 2015 at 09:53:50AM +0100, Rasmus Villemoes wrote:

> It seems that gcc happily compiles
> 
> for (i = 0; i < 1000000000; ++i) ;
> 
> into simply
> 
> i = 1000000000;
> 
> (which is then usually eliminated as a dead store). At least at -O2, and
> when i is not declared volatile. So it would seem that the loops at
> 
> arch/mips/pci/pci-rt2880.c:235
> arch/mips/pmcs-msp71xx/msp_setup.c:80
> arch/mips/sni/reset.c:35
> 
> actually don't do anything. (In the middle one, i is 'register', but
> that doesn't change anything.) Is mips compiled with some special flags
> that would make gcc actually emit code for the above?

Thanks for reporting!

GCC used to intentionally not eleminate empty loops.  This has changed a
while ago.  Using volatile for the loop variable will result in atrocious
code so should be avoided.

One problem of these open coded loops is that it's not obvious how much
delay was actually intended so when fixing this I will have to do a bit
of precission guessing ;-)

  Ralf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web