don't use char for defining PRPackedBool when compiling with GCC

RESOLVED WONTFIX

Status

--
enhancement
RESOLVED WONTFIX
16 years ago
16 years ago

People

(Reporter: dann, Assigned: wtc)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

16 years ago
Currently in prtypes.h PRPackedBool is defined to be an unsigned char. 

This is inefficient as it disables type based aliasing optimizations.

For example, the following simple code: 

typedef unsigned char PRPackedBool;

void
foo (PRPackedBool *b, int *p)
{
  (*p)++;
  (*b) = 0;
  (*p)++;
  (*b) = 1;
}

one would expect that the first assingment to *b to be eliminated and *p to be
incremented by 2. 
but the code is compiled by gcc-3.1 -O2 on a SPARC machine to: 

foo:
        ld      [%o1], %o2
        add     %o2, 1, %o2
        st      %o2, [%o1]
        stb     %g0, [%o0]
        ld      [%o1], %o2
        add     %o2, 1, %o2
        st      %o2, [%o1]
        mov     1, %o1
        retl
        stb     %o1, [%o0]

so there's 4 loads, 4 stores, 2 adds ...
This is because of the bad aliasing properties of char*

The same type of code is generated by the SUN compiler on sparc, and by gcc,
Intel's compiler and MS' compiler on x86

It the typedef is changed to: 

typedef _Bool PRPackedBool;

then the generated code is much better: 

        ld      [%o1], %o2
        add     %o2, 2, %o2
        st      %o2, [%o1]
        mov     1, %o1
        retl
        stb     %o1, [%o0]

So the proposal is to replace the definition of PRPackedBool with something like:


#ifndef __cplusplus
typedef _Bool PRPackedBool;
#else
typedef bool PRPackedBool;
#endif

this might also need some autoconf tests because "bool" was not 1 byte for all
the GCC versions. 

Comments?
(Assignee)

Comment 1

16 years ago
Thank you very much for the suggestion.

This is a good idea, but unfortunately I can't
make this change.  NSPR needs to maintain backward
compatibility at the binary level.  This would
require:

1. _Bool and bool be one byte;
2. _Bool and bool behave exactly like unsigned char.

Neither of these is true.

Perhaps you can regain the compiler optimization by
using the new 'restrict' type qualifier of C99.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 2

16 years ago
You said: 
  > 1. _Bool and bool be one byte;
  > 2. _Bool and bool behave exactly like unsigned char.

  > Neither of these is true.

  > Perhaps you can regain the compiler optimization by
  > using the new 'restrict' type qualifier of C99.

What makes you think that Bool and bool are not one byte? 
bool used to be 4 bytes in gcc-2.8 but it has been changed to 1 byte after 
egcs-1.x

What do you mean by "behave exactly like unsigned char" ? What type of behavior 
do you have in mind? 

No, restrict is not going to help in the above example. Anything in memory is
supposed to be addressable using a char*.

Note that I have not tested the impact that the proposed change will have on the
mozilla code base, but as a matter of principle it is not a good idea to 
not tell the compiler the real type of a variable, it will always disable 
some optimizations. And the number of optimizations that only apply to boolean
types will only increase with time. 

So please reconsider your decision, or at least put it up for more debate with 
the other mozilla developers. 

You need to log in before you can comment on or make changes to this bug.