public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
@ 2002-04-16  2:19 Frank Davis
  2002-04-17  2:47 ` Adam Kropelin
  2002-04-21 13:37 ` [PATCH] " Brian Gerst
  0 siblings, 2 replies; 19+ messages in thread
From: Frank Davis @ 2002-04-16  2:19 UTC (permalink / raw)
  To: linux-kernel; +Cc: davej, fdavis

Hello all,
  While a 'make bzImage', I received the following compile error:
Regards,
Frank

smpboot.c:1008: parse error before `0'
smpboot.c: In function `smp_boot_cpus':
smpboot.c:1023: invalid lvalue in assignment
make[1]: *** [smpboot.o] Error 1
make[1]: Leaving directory `/usr/src/linux/arch/i386/kernel'
make: *** [_dir_arch/i386/kernel] Error 2



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-16  2:19 2.5.8-dj1 : arch/i386/kernel/smpboot.c error Frank Davis
@ 2002-04-17  2:47 ` Adam Kropelin
  2002-04-17  4:59   ` Martin J. Bligh
  2002-04-21 13:37 ` [PATCH] " Brian Gerst
  1 sibling, 1 reply; 19+ messages in thread
From: Adam Kropelin @ 2002-04-17  2:47 UTC (permalink / raw)
  To: Frank Davis; +Cc: linux-kernel, davej

On Mon, Apr 15, 2002 at 10:19:24PM -0400, Frank Davis wrote:
> Hello all,
>   While a 'make bzImage', I received the following compile error:
> Regards,
> Frank
> 
> smpboot.c:1008: parse error before `0'
> smpboot.c: In function `smp_boot_cpus':
> smpboot.c:1023: invalid lvalue in assignment
> make[1]: *** [smpboot.o] Error 1
> make[1]: Leaving directory `/usr/src/linux/arch/i386/kernel'
> make: *** [_dir_arch/i386/kernel] Error 2

There's an optimization in io.h for !CONFIG_MULTIQUAD which doesn't seem to have been
carried through all the way...

The following patch seems logical, but I'm no expert. In particular, I'm not sure
if the ioremapping bit ever needs to be run when !CONFIG_MULTIQUAD...

--Adam

--- linux-2.5.8-dj1-virgin/arch/i386/kernel/smpboot.c	Tue Apr 16 16:21:24 2002
+++ linux-2.5.8-dj1+smpboot-fix/arch/i386/kernel/smpboot.c	Tue Apr 16 21:12:13 2002
@@ -1004,8 +1004,11 @@
 extern int prof_counter[NR_CPUS];
 
 static int boot_cpu_logical_apicid;
+
 /* Where the IO area was mapped on multiquad, always 0 otherwise */
+#ifdef CONFIG_MULTIQUAD
 void *xquad_portio = NULL;
+#endif
 
 int cpu_sibling_map[NR_CPUS] __cacheline_aligned;
 
@@ -1013,6 +1016,7 @@
 {
 	int apicid, cpu, bit;
 
+#ifdef CONFIG_MULTIQUAD
         if (clustered_apic_mode && (numnodes > 1)) {
                 printk("Remapping cross-quad port I/O for %d quads\n",
 			numnodes);
@@ -1022,6 +1026,7 @@
                 xquad_portio = ioremap (XQUAD_PORTIO_BASE, 
 			numnodes * XQUAD_PORTIO_LEN);
         }
+#endif
 
 #ifdef CONFIG_MTRR
 	/*  Must be done before other processors booted  */


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17  2:47 ` Adam Kropelin
@ 2002-04-17  4:59   ` Martin J. Bligh
  2002-04-17 12:30     ` Adam Kropelin
  0 siblings, 1 reply; 19+ messages in thread
From: Martin J. Bligh @ 2002-04-17  4:59 UTC (permalink / raw)
  To: Adam Kropelin, Frank Davis; +Cc: linux-kernel, davej

xquad_portio is indeed only for CONFIG_MULTIQUAD. However, you
shouldn't need the #ifdef's in the code to make this work -
clustered_apic_mode isn't a variable at all, it's a magic
trick that's actually 1 or 0 depending on CONFIG_MULTIQUAD.

Look at 2.5.8 virgin, it has the same code.

M.

--On Tuesday, April 16, 2002 10:47 PM -0400 Adam Kropelin
<akropel1@rochester.rr.com> wrote:

> On Mon, Apr 15, 2002 at 10:19:24PM -0400, Frank Davis wrote:
>> Hello all,
>>   While a 'make bzImage', I received the following compile error:
>> Regards,
>> Frank
>> 
>> smpboot.c:1008: parse error before `0'
>> smpboot.c: In function `smp_boot_cpus':
>> smpboot.c:1023: invalid lvalue in assignment
>> make[1]: *** [smpboot.o] Error 1
>> make[1]: Leaving directory `/usr/src/linux/arch/i386/kernel'
>> make: *** [_dir_arch/i386/kernel] Error 2
> 
> There's an optimization in io.h for !CONFIG_MULTIQUAD which doesn't seem
> to have been carried through all the way...
> 
> The following patch seems logical, but I'm no expert. In particular, I'm
> not sure if the ioremapping bit ever needs to be run when
> !CONFIG_MULTIQUAD...
> 
> --Adam
> 
> --- linux-2.5.8-dj1-virgin/arch/i386/kernel/smpboot.c	Tue Apr 16 16:21:24
> 2002 +++ linux-2.5.8-dj1+smpboot-fix/arch/i386/kernel/smpboot.c	Tue Apr
> 16 21:12:13 2002 @@ -1004,8 +1004,11 @@
>  extern int prof_counter[NR_CPUS];
>  
>  static int boot_cpu_logical_apicid;
> +
>  /* Where the IO area was mapped on multiquad, always 0 otherwise */
> +#ifdef CONFIG_MULTIQUAD
>  void *xquad_portio = NULL;
> +#endif
>  
>  int cpu_sibling_map[NR_CPUS] __cacheline_aligned;
>  
> @@ -1013,6 +1016,7 @@
>  {
>  	int apicid, cpu, bit;
>  
> +#ifdef CONFIG_MULTIQUAD
>          if (clustered_apic_mode && (numnodes > 1)) {
>                  printk("Remapping cross-quad port I/O for %d quads\n",
>  			numnodes);
> @@ -1022,6 +1026,7 @@
>                  xquad_portio = ioremap (XQUAD_PORTIO_BASE, 
>  			numnodes * XQUAD_PORTIO_LEN);
>          }
> +#endif
>  
>  #ifdef CONFIG_MTRR
>  	/*  Must be done before other processors booted  */
> 
> -
> 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/
> 



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17  4:59   ` Martin J. Bligh
@ 2002-04-17 12:30     ` Adam Kropelin
  2002-04-17 12:40       ` Adam Kropelin
  2002-04-17 15:28       ` Martin J. Bligh
  0 siblings, 2 replies; 19+ messages in thread
From: Adam Kropelin @ 2002-04-17 12:30 UTC (permalink / raw)
  To: Martin J. Bligh; +Cc: Frank Davis, linux-kernel, davej

On Tue, Apr 16, 2002 at 09:59:08PM -0700, Martin J. Bligh wrote:
> xquad_portio is indeed only for CONFIG_MULTIQUAD. However, you
> shouldn't need the #ifdef's in the code to make this work -
> clustered_apic_mode isn't a variable at all, it's a magic
> trick that's actually 1 or 0 depending on CONFIG_MULTIQUAD.
> 
> Look at 2.5.8 virgin, it has the same code.

Not quite.

As I said, -dj has an optimization in asm-i386/io.o:

> #ifdef CONFIG_MULTIQUAD
> extern void *xquad_portio;    /* Where the IO area was mapped */
> #else
> #define xquad_portio (0)
> #endif

So the preprocessed smpboot.c contains gems like:

> void *(0) = ((void *)0);

...and...

> (0) = ioremap (0xfe400000,
>         numnodes * 0x80000);

Even though clustered_apic_mode is 0, the compiler still complains
about the second one and the first one doesn't depend on
clustered_apic_mode at all.

I don't like spreading around more #ifdef's, but the spirit of the
changes seemed to be to get rid of the declaration of xquad_portio
when !CONFIG_MULTIQUAD. Suggestions for improvement welcome.

--Adam


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 12:30     ` Adam Kropelin
@ 2002-04-17 12:40       ` Adam Kropelin
  2002-04-17 15:28       ` Martin J. Bligh
  1 sibling, 0 replies; 19+ messages in thread
From: Adam Kropelin @ 2002-04-17 12:40 UTC (permalink / raw)
  To: Martin J. Bligh; +Cc: Frank Davis, linux-kernel, davej

On Wed, Apr 17, 2002 at 08:30:44AM -0400, Adam Kropelin wrote:
> As I said, -dj has an optimization in asm-i386/io.o:
                                                    ^
io.h, of course...

--Adam


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 12:30     ` Adam Kropelin
  2002-04-17 12:40       ` Adam Kropelin
@ 2002-04-17 15:28       ` Martin J. Bligh
  2002-04-17 19:17         ` Adam Kropelin
  2002-04-17 22:56         ` Stevie O
  1 sibling, 2 replies; 19+ messages in thread
From: Martin J. Bligh @ 2002-04-17 15:28 UTC (permalink / raw)
  To: Adam Kropelin; +Cc: Frank Davis, linux-kernel, davej, Brian Gerst

cc'ed Brian - this is your io.h cleanup patch

> As I said, -dj has an optimization in asm-i386/io.o:
> 
>> # ifdef CONFIG_MULTIQUAD
>> extern void *xquad_portio;    /* Where the IO area was mapped */
>> # else
>> # define xquad_portio (0)
>> # endif

Ah, OK, I missed that - makes more sense now ;-). 

> Even though clustered_apic_mode is 0, the compiler still complains
> about the second one and the first one doesn't depend on
> clustered_apic_mode at all.

Hmmm ... not sure why the compiler complains about the second one,
that's very strange ;-)
 
> I don't like spreading around more #ifdef's, but the spirit of the
> changes seemed to be to get rid of the declaration of xquad_portio
> when !CONFIG_MULTIQUAD. Suggestions for improvement welcome.

The cleanups we gained in io.h by Brian's patch more than compensate
for this, but it's still a shame to have to do.

I wonder if we can play the same trick we've played before ....
haven't tested the appended, but maybe it, or something like it
will work without the ifdef's?

M.

--- linux-2.5.8-dj1/include/asm-i386/io.h	Wed Apr 17 05:06:20 2002
+++ linux-2.5.8-dj1/include/asm-i386/io.h.new	Wed Apr 17 15:07:11 2002
@@ -330,7 +330,8 @@
 }
 
 #ifdef CONFIG_MULTIQUAD
-extern void *xquad_portio;    /* Where the IO area was mapped */
+#define xquad_portio real_xquad_portio
+extern void *real_xquad_portio;    /* Where the IO area was mapped */
 #else
 #define xquad_portio (0)
 #endif
--- linux-2.5.8-dj1/arch/i386/kernel/smpboot.c	Sun Apr 14 12:18:52 2002
+++ linux-2.5.8-dj1/arch/i386/kernel/smpboot.c.new	Wed Apr 17 15:08:25 2002
@@ -1005,7 +1005,7 @@
 
 static int boot_cpu_logical_apicid;
 /* Where the IO area was mapped on multiquad, always 0 otherwise */
-void *xquad_portio = NULL;
+void *real_xquad_portio = NULL;
 
 int cpu_sibling_map[NR_CPUS] __cacheline_aligned;
 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 15:28       ` Martin J. Bligh
@ 2002-04-17 19:17         ` Adam Kropelin
  2002-04-17 19:31           ` Rick Stevens
  2002-04-17 20:49           ` Martin J. Bligh
  2002-04-17 22:56         ` Stevie O
  1 sibling, 2 replies; 19+ messages in thread
From: Adam Kropelin @ 2002-04-17 19:17 UTC (permalink / raw)
  To: Martin J. Bligh; +Cc: Frank Davis, linux-kernel, davej, Brian Gerst

On Wed, Apr 17, 2002 at 08:28:19AM -0700, Martin J. Bligh wrote:
> Adam Kropelin wrote:
> > Even though clustered_apic_mode is 0, the compiler still complains
> > about the second one and the first one doesn't depend on
> > clustered_apic_mode at all.
> 
> Hmmm ... not sure why the compiler complains about the second one,
> that's very strange ;-)

I agree. The cpp ouput clealy shows

        if ((0) && (numnodes > 1)) {

so I'm not sure why there's a problem.

> I wonder if we can play the same trick we've played before ....
> haven't tested the appended, but maybe it, or something like it
> will work without the ifdef's?

IMHO, this sort of trickery in the name of improving readability
is misguided. To me, anyway, the #ifdef's are much easer to read than
magic name-changing macros buried in a header somewhere.

--Adam

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 19:17         ` Adam Kropelin
@ 2002-04-17 19:31           ` Rick Stevens
  2002-04-17 20:53             ` Martin J. Bligh
  2002-04-17 20:49           ` Martin J. Bligh
  1 sibling, 1 reply; 19+ messages in thread
From: Rick Stevens @ 2002-04-17 19:31 UTC (permalink / raw)
  To: linux-kernel

Adam Kropelin wrote:
> On Wed, Apr 17, 2002 at 08:28:19AM -0700, Martin J. Bligh wrote:
> 
>>Adam Kropelin wrote:
>>
>>>Even though clustered_apic_mode is 0, the compiler still complains
>>>about the second one and the first one doesn't depend on
>>>clustered_apic_mode at all.
>>>
>>Hmmm ... not sure why the compiler complains about the second one,
>>that's very strange ;-)
>>
> 
> I agree. The cpp ouput clealy shows
> 
>         if ((0) && (numnodes > 1)) {
> 
> so I'm not sure why there's a problem.

Is the thing generating the "(0)" a macro?  If so, then the rules
of C and the "&&" operator say that "if the first is false, the
second needn't even be evaluated".  Could that be what's causing
the warning?
----------------------------------------------------------------------
- Rick Stevens, SSE, VitalStream, Inc.      rstevens@vitalstream.com -
- 949-743-2010 (Voice)                    http://www.vitalstream.com -
-                                                                    -
-         We have enough youth, how about a fountain of SMART?       -
----------------------------------------------------------------------


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 20:53             ` Martin J. Bligh
@ 2002-04-17 20:40               ` Adam Kropelin
  2002-04-17 21:17                 ` Rick Stevens
  2002-04-18  0:34                 ` Martin J. Bligh
  0 siblings, 2 replies; 19+ messages in thread
From: Adam Kropelin @ 2002-04-17 20:40 UTC (permalink / raw)
  To: Martin J. Bligh; +Cc: Rick Stevens, linux-kernel

On Wed, Apr 17, 2002 at 01:53:55PM -0700, Martin J. Bligh wrote:
> > of C and the "&&" operator say that "if the first is false, the
> > second needn't even be evaluated".  
> 
> That's what I would have thought.
> But I don't think it's the second part that causes the warning,
> it's the thing *inside* the if clause.

Exactly.

> > Could that be what's causing the warning?
> 
> To my mind, that's why we should *not* be getting a warning ?

Indeed. The optimization step that (presumably) removes the body
of the if() must happen after the body has been fully evaluated.
Makes sense, I guess, now that I think about it...

--Adam

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 19:17         ` Adam Kropelin
  2002-04-17 19:31           ` Rick Stevens
@ 2002-04-17 20:49           ` Martin J. Bligh
  1 sibling, 0 replies; 19+ messages in thread
From: Martin J. Bligh @ 2002-04-17 20:49 UTC (permalink / raw)
  To: Adam Kropelin; +Cc: Frank Davis, linux-kernel, davej, Brian Gerst

>> I wonder if we can play the same trick we've played before ....
>> haven't tested the appended, but maybe it, or something like it
>> will work without the ifdef's?
> 
> IMHO, this sort of trickery in the name of improving readability
> is misguided. To me, anyway, the #ifdef's are much easer to read than
> magic name-changing macros buried in a header somewhere.

Well, except that you can take that abstraction inside your head, and
not worry about it when reading the mainline code. I don't really care
one way or the other, at least io.h is readable now ;-)

M.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 19:31           ` Rick Stevens
@ 2002-04-17 20:53             ` Martin J. Bligh
  2002-04-17 20:40               ` Adam Kropelin
  0 siblings, 1 reply; 19+ messages in thread
From: Martin J. Bligh @ 2002-04-17 20:53 UTC (permalink / raw)
  To: Rick Stevens, linux-kernel

>>>> Even though clustered_apic_mode is 0, the compiler still complains
>>>> about the second one and the first one doesn't depend on
>>>> clustered_apic_mode at all.
>>>> 
>>> Hmmm ... not sure why the compiler complains about the second one,
>>> that's very strange ;-)
>>> 
>> 
>> I agree. The cpp ouput clealy shows
>> 
>>         if ((0) && (numnodes > 1)) {
>> 
>> so I'm not sure why there's a problem.
> 
> Is the thing generating the "(0)" a macro?  If so, then the rules

  #define clustered_apic_mode (0)

> of C and the "&&" operator say that "if the first is false, the
> second needn't even be evaluated".  

That's what I would have thought.
But I don't think it's the second part that causes the warning,
it's the thing *inside* the if clause.

> Could that be what's causing the warning?

To my mind, that's why we should *not* be getting a warning ?

M.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 20:40               ` Adam Kropelin
@ 2002-04-17 21:17                 ` Rick Stevens
  2002-04-17 21:49                   ` Adam Kropelin
  2002-04-18  0:34                 ` Martin J. Bligh
  1 sibling, 1 reply; 19+ messages in thread
From: Rick Stevens @ 2002-04-17 21:17 UTC (permalink / raw)
  To: linux-kernel

Adam Kropelin wrote:
> On Wed, Apr 17, 2002 at 01:53:55PM -0700, Martin J. Bligh wrote:
> 
>>>of C and the "&&" operator say that "if the first is false, the
>>>second needn't even be evaluated".  
>>>
>>That's what I would have thought.
>>But I don't think it's the second part that causes the warning,
>>it's the thing *inside* the if clause.
>>
> 
> Exactly.
> 
> 
>>>Could that be what's causing the warning?
>>>
>>To my mind, that's why we should *not* be getting a warning ?
>>
> 
> Indeed. The optimization step that (presumably) removes the body
> of the if() must happen after the body has been fully evaluated.
> Makes sense, I guess, now that I think about it...

Right.  If the first condition of a logical AND statement is false,
the remainder need not be evaluated at all.  Hence, the entire if
statement can (and perhaps should) be eliminated by the compiler,
since the condition is false.

I didn't see what the actual message from the compiler was, but it
was probably just a warning.
----------------------------------------------------------------------
- Rick Stevens, SSE, VitalStream, Inc.      rstevens@vitalstream.com -
- 949-743-2010 (Voice)                    http://www.vitalstream.com -
-                                                                    -
-     Never put off 'til tommorrow what you can forget altogether!   -
----------------------------------------------------------------------


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 21:17                 ` Rick Stevens
@ 2002-04-17 21:49                   ` Adam Kropelin
  2002-04-17 22:06                     ` J.A. Magallon
  0 siblings, 1 reply; 19+ messages in thread
From: Adam Kropelin @ 2002-04-17 21:49 UTC (permalink / raw)
  To: Rick Stevens, linux-kernel

Rick Stevens wrote:
> Adam Kropelin wrote:
>> Indeed. The optimization step that (presumably) removes the body
>> of the if() must happen after the body has been fully evaluated.
>> Makes sense, I guess, now that I think about it...
> 
> Right.  If the first condition of a logical AND statement is false,
> the remainder need not be evaluated at all.  Hence, the entire if
> statement can (and perhaps should) be eliminated by the compiler,
> since the condition is false.
> 
> I didn't see what the actual message from the compiler was, but it
> was probably just a warning.

Thanks for the help, but it was an error, not a warning.

It is caused by invalid code inside the body of the if(). The question
is not what is causing the error but why the compiler is evaluating the
body at all, given that the conditional is always false.

Apparently the body is evaluated prior to the optimizer getting a 
chance to nullify the whole construct.

--Adam



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 21:49                   ` Adam Kropelin
@ 2002-04-17 22:06                     ` J.A. Magallon
  0 siblings, 0 replies; 19+ messages in thread
From: J.A. Magallon @ 2002-04-17 22:06 UTC (permalink / raw)
  To: Adam Kropelin; +Cc: Rick Stevens, linux-kernel


On 2002.04.17 Adam Kropelin wrote:
>Rick Stevens wrote:
>> Adam Kropelin wrote:
>>> Indeed. The optimization step that (presumably) removes the body
>>> of the if() must happen after the body has been fully evaluated.
>>> Makes sense, I guess, now that I think about it...
>> 
>> Right.  If the first condition of a logical AND statement is false,
>> the remainder need not be evaluated at all.  Hence, the entire if
>> statement can (and perhaps should) be eliminated by the compiler,
>> since the condition is false.
>> 
>> I didn't see what the actual message from the compiler was, but it
>> was probably just a warning.
>
>Thanks for the help, but it was an error, not a warning.
>
>It is caused by invalid code inside the body of the if(). The question
>is not what is causing the error but why the compiler is evaluating the
>body at all, given that the conditional is always false.
>
>Apparently the body is evaluated prior to the optimizer getting a 
>chance to nullify the whole construct.
>

AFAIK, you first parse C to build a tree of commands, expresions, etc,
and then you do the rest on the tree (optimizations at source-code level,
like CSE, ie, detection of equal subtrees, or for example
killig and if tree if it is always false).

Hope this helps.

-- 
J.A. Magallon                           #  Let the source be with you...        
mailto:jamagallon@able.es
Mandrake Linux release 8.3 (Cooker) for i586
Linux werewolf 2.4.19-pre7-jam1 #2 SMP Wed Apr 17 21:20:31 CEST 2002 i686

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 15:28       ` Martin J. Bligh
  2002-04-17 19:17         ` Adam Kropelin
@ 2002-04-17 22:56         ` Stevie O
  2002-04-18  0:29           ` Martin J. Bligh
  1 sibling, 1 reply; 19+ messages in thread
From: Stevie O @ 2002-04-17 22:56 UTC (permalink / raw)
  To: Martin J. Bligh, Adam Kropelin
  Cc: Frank Davis, linux-kernel, davej, Brian Gerst

At 08:28 AM 4/17/2002 -0700, Martin J. Bligh wrote:
>> Even though clustered_apic_mode is 0, the compiler still complains
>> about the second one and the first one doesn't depend on
>> clustered_apic_mode at all.
>
>Hmmm ... not sure why the compiler complains about the second one,
>that's very strange ;-)

That's because we're using C. If we rewrote the kernel in FORTRAN, the FORTRAN compiler would happily let us redefine 0 to any other value :)

(How bout it guys? We could rewrite the kernel in C++, then run a C++-to-FORTRAN conversion script on it!)


--
Stevie-O

Real programmers use COPY CON PROGRAM.EXE


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 22:56         ` Stevie O
@ 2002-04-18  0:29           ` Martin J. Bligh
  0 siblings, 0 replies; 19+ messages in thread
From: Martin J. Bligh @ 2002-04-18  0:29 UTC (permalink / raw)
  To: Stevie O, Adam Kropelin; +Cc: Frank Davis, linux-kernel, davej, Brian Gerst

>>> Even though clustered_apic_mode is 0, the compiler still complains
>>> about the second one and the first one doesn't depend on
>>> clustered_apic_mode at all.
>> 
>> Hmmm ... not sure why the compiler complains about the second one,
>> that's very strange ;-)
> 
> That's because we're using C. If we rewrote the kernel in FORTRAN, the
> FORTRAN compiler would happily let us redefine 0 to any other value :)

I think you're missing the point. It shouldn't be compiling that whole if
clause
at all, as far as I can see.

M.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-17 20:40               ` Adam Kropelin
  2002-04-17 21:17                 ` Rick Stevens
@ 2002-04-18  0:34                 ` Martin J. Bligh
  2002-04-18  2:16                   ` Stevie O
  1 sibling, 1 reply; 19+ messages in thread
From: Martin J. Bligh @ 2002-04-18  0:34 UTC (permalink / raw)
  To: Adam Kropelin; +Cc: Rick Stevens, linux-kernel

> Indeed. The optimization step that (presumably) removes the body
> of the if() must happen after the body has been fully evaluated.
> Makes sense, I guess, now that I think about it...

Personally, I think that's a really sick and twisted way for a compiler
to work ... what the hell is the point of compiling something you know
perfectly well you're going to dispose of 2 nanoseconds later?

M.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-18  0:34                 ` Martin J. Bligh
@ 2002-04-18  2:16                   ` Stevie O
  0 siblings, 0 replies; 19+ messages in thread
From: Stevie O @ 2002-04-18  2:16 UTC (permalink / raw)
  To: Martin J. Bligh, Adam Kropelin; +Cc: Rick Stevens, linux-kernel

At 05:34 PM 4/17/2002 -0700, Martin J. Bligh wrote:

>Personally, I think that's a really sick and twisted way for a compiler
>to work ... what the hell is the point of compiling something you know
>perfectly well you're going to dispose of 2 nanoseconds later?
>
>M.

<RANT>

Granted, I could make a rather similar argument for the reverse: What the hell is the point of giving something to the compiler if we know perfectly well that it would be optimized out anyway?

(Especially if the alternative, leaving it up to the compiler, generates totally insane code like '0 = f();')

When I downloaded and put together 2.5.7, I couldn't even build the kernel because I didn't compile NFS support in ("undefined ref to sys_nfsservctl"); I dread to think of the number of errors I'd see if the compiler's optimizer dropped out blatantly erroneous code because it happened to be surrounded by 'if (0 && other_func())' and optimized out on certain builds.

Because if:
        (void*) 0 = (void*) 0;

isn't blatantly erroneous (even if not evident at first hand), I don't know what is.

And if the fact that it decomposed into "0 = 0;" wasn't blatantly obvious, all the more reason for my next point:

----

Yes, I agree that as code becomes #ifdef-laden, it becomes hard to read -- at an exponential rate,
especially if it's poorly done:

#ifdef OPTION_X
        x 
#else
#ifdef OPTION_Y
        y
#else
        z
#endif
#endif
                =
#ifdef OPTION_X
        x_func(x,
#else
        yz_func(
#ifdef OPTION_Y
                y
#else
                z
#endif
                 ,
#endif
                  32);

Yes, I've seen beautiful code mauled by nested #if's and #ifdef's and such.

However... Which of the following is more obviously code geared towards a specific config option?

#ifdef CONFIG_OBSCURE
  if (obscure_variable == 0) {
        obscure_variable = init_obscure_variable_function();
  }
#endif

-or-

  if (obscure_variable == 0) {
        obscure_variable = init_obscure_variable_function();
  }


If I didn't know that 'obscure_variable' was #defined to 0 in 'obscure_header.h':

#ifdef CONFIG_OBSCURE
extern void* obscure_variable;
#else
#define obscure_variable ((void*)0)
#endif

I might wonder why my system never links in 'init_obscure_variable_function' despite the reference.

The preprocessor is a tool.  It's designed to help us.  Just because it's misused often doesn't mean we need to reject it in every situation.  (Just like guns, the preprocessor will let you shoot yourself in the foot! :P)


Okay, I'm out of breath for now.
</RANT>


--
Stevie-O

Real programmers write code like this:

const char *FlashPROMFunc;

...
 (((void)(*)(void*,void*,int,int))(FlashPROMFunc))(dst, src, count, 1);

[yes, I wrote that line of code once. yes, it was for production use. yes, it worked.]


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] Re: 2.5.8-dj1 : arch/i386/kernel/smpboot.c error
  2002-04-16  2:19 2.5.8-dj1 : arch/i386/kernel/smpboot.c error Frank Davis
  2002-04-17  2:47 ` Adam Kropelin
@ 2002-04-21 13:37 ` Brian Gerst
  1 sibling, 0 replies; 19+ messages in thread
From: Brian Gerst @ 2002-04-21 13:37 UTC (permalink / raw)
  To: Frank Davis; +Cc: linux-kernel, davej, Martin.Bligh

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

Frank Davis wrote:
> Hello all,
>   While a 'make bzImage', I received the following compile error:
> Regards,
> Frank
> 
> smpboot.c:1008: parse error before `0'
> smpboot.c: In function `smp_boot_cpus':
> smpboot.c:1023: invalid lvalue in assignment
> make[1]: *** [smpboot.o] Error 1
> make[1]: Leaving directory `/usr/src/linux/arch/i386/kernel'
> make: *** [_dir_arch/i386/kernel] Error 2
> 
> 
> -
> 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/
> 

I reworked the patch to avoid the #define of xquad_portio.  It is still 
much more readable than the original code.  Patch attached against 
2.5.8-dj1.

-- 

						Brian Gerst

[-- Attachment #2: io-4 --]
[-- Type: text/plain, Size: 3033 bytes --]

diff -urN linux-2.5.8-dj1/include/asm-i386/io.h linux/include/asm-i386/io.h
--- linux-2.5.8-dj1/include/asm-i386/io.h	Sun Apr 21 09:14:24 2002
+++ linux/include/asm-i386/io.h	Sun Apr 21 09:27:20 2002
@@ -331,16 +331,8 @@
 
 #ifdef CONFIG_MULTIQUAD
 extern void *xquad_portio;    /* Where the IO area was mapped */
-#else
-#define xquad_portio (0)
-#endif
-
 #define XQUAD_PORT_ADDR(port, quad) (xquad_portio + (XQUAD_PORTIO_QUAD*quad) + port)
-
 #define __BUILDIO(bwl,bw,type) \
-static inline void out##bwl##_local(unsigned type value, int port) { \
-	__asm__ __volatile__("out" #bwl " %" #bw "0, %w1" : : "a"(value), "Nd"(port)); \
-} \
 static inline void out##bwl##_quad(unsigned type value, int port, int quad) { \
 	if (xquad_portio) \
 		write##bwl(value, XQUAD_PORT_ADDR(port, quad)); \
@@ -350,18 +342,6 @@
 static inline void out##bwl(unsigned type value, int port) { \
 	out##bwl##_quad(value, port, 0); \
 } \
-static inline void out##bwl##_p(unsigned type value, int port) { \
-	out##bwl(value, port); \
-	slow_down_io(); \
-} \
-static inline void outs##bwl(int port, const void *addr, unsigned long count) { \
-	__asm__ __volatile__("rep; outs" #bwl : "+S"(addr), "+c"(count) : "d"(port)); \
-} \
-static inline unsigned type in##bwl##_local(int port) { \
-	unsigned type value; \
-	__asm__ __volatile__("in" #bwl " %w1, %" #bw "0" : "=a"(value) : "Nd"(port)); \
-	return value; \
-} \
 static inline unsigned type in##bwl##_quad(int port, int quad) { \
 	if (xquad_portio) \
 		return read##bwl(XQUAD_PORT_ADDR(port, quad)); \
@@ -370,18 +350,46 @@
 } \
 static inline unsigned type in##bwl(int port) { \
 	return in##bwl##_quad(port, 0); \
+}
+#else
+#define __BUILDIO(bwl,bw,type) \
+static inline void out##bwl(unsigned type value, int port) { \
+	out##bwl##_local(value, port); \
+} \
+static inline unsigned type in##bwl(int port) { \
+	return in##bwl##_local(port); \
+}
+#endif
+
+
+#define BUILDIO(bwl,bw,type) \
+static inline void out##bwl##_local(unsigned type value, int port) { \
+	__asm__ __volatile__("out" #bwl " %" #bw "0, %w1" : : "a"(value), "Nd"(port)); \
+} \
+static inline unsigned type in##bwl##_local(int port) { \
+	unsigned type value; \
+	__asm__ __volatile__("in" #bwl " %w1, %" #bw "0" : "=a"(value) : "Nd"(port)); \
+	return value; \
+} \
+__BUILDIO(bwl,bw,type) \
+static inline void out##bwl##_p(unsigned type value, int port) { \
+	out##bwl(value, port); \
+	slow_down_io(); \
 } \
 static inline unsigned type in##bwl##_p(int port) { \
 	unsigned type value = in##bwl(port); \
 	slow_down_io(); \
 	return value; \
 } \
+static inline void outs##bwl(int port, const void *addr, unsigned long count) { \
+	__asm__ __volatile__("rep; outs" #bwl : "+S"(addr), "+c"(count) : "d"(port)); \
+} \
 static inline void ins##bwl(int port, void *addr, unsigned long count) { \
 	__asm__ __volatile__("rep; ins" #bwl : "+D"(addr), "+c"(count) : "d"(port)); \
 }
 
-__BUILDIO(b,b,char)
-__BUILDIO(w,w,short)
-__BUILDIO(l,,long)
+BUILDIO(b,b,char)
+BUILDIO(w,w,short)
+BUILDIO(l,,long)
 
 #endif

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2002-04-21 13:39 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-04-16  2:19 2.5.8-dj1 : arch/i386/kernel/smpboot.c error Frank Davis
2002-04-17  2:47 ` Adam Kropelin
2002-04-17  4:59   ` Martin J. Bligh
2002-04-17 12:30     ` Adam Kropelin
2002-04-17 12:40       ` Adam Kropelin
2002-04-17 15:28       ` Martin J. Bligh
2002-04-17 19:17         ` Adam Kropelin
2002-04-17 19:31           ` Rick Stevens
2002-04-17 20:53             ` Martin J. Bligh
2002-04-17 20:40               ` Adam Kropelin
2002-04-17 21:17                 ` Rick Stevens
2002-04-17 21:49                   ` Adam Kropelin
2002-04-17 22:06                     ` J.A. Magallon
2002-04-18  0:34                 ` Martin J. Bligh
2002-04-18  2:16                   ` Stevie O
2002-04-17 20:49           ` Martin J. Bligh
2002-04-17 22:56         ` Stevie O
2002-04-18  0:29           ` Martin J. Bligh
2002-04-21 13:37 ` [PATCH] " Brian Gerst

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox