size of pointer and size of long

In the recent changes to the preprocessor, new assumptions are added
that assume the size of pointer to be same as size of long (or size of
long long for 64 bit targets)
Although this is to mimic gcc behavior, not all ports necessarily want
to follow suite.
Our target does not support long long and assumes 32 bit long and 16 bit
pointer.

There is an assertion in the Preprocessor.cpp line 600 that assumes
current targets have sizeof(long) == sizeof(void *). This assumption is
incorrect.

Are these assumptions only in the preprocessor? Or there are other
places in clang that these assumptions are also made?

Regards,
Alireza Moshtaghi
Senior Software Engineer
Development Systems, Microchip Technology

In the recent changes to the preprocessor, new assumptions are added
that assume the size of pointer to be same as size of long (or size of
long long for 64 bit targets)
Although this is to mimic gcc behavior, not all ports necessarily want
to follow suite.

Definitely... that isn't intended to be a permanent restriction. In
addition to 16-bit targets, Win64 breaks that assumption.

Our target does not support long long and assumes 32 bit long and 16 bit
pointer.

Okay, good to know; so for PIC16, the correct specification is that
everything has alignment of 8 bits, int and pointer have size 16 bits,
long has size 32 bits, and long long is illegal? While I'm asking, is
there any floating-point support?

There is an assertion in the Preprocessor.cpp line 600 that assumes
current targets have sizeof(long) == sizeof(void *). This assumption is
incorrect.

Yeah; the only reason Chris did that was to avoid adding a few extra
cases to the code. Feel free to submit a patch, or else I'm sure
Chris will get to it soon.

Are these assumptions only in the preprocessor? Or there are other
places in clang that these assumptions are also made?

There are a few places scattered through the code making bad
assumptions about types because target data didn't have sufficient
information, although not many; we've been pretty good about making
Sema match the actual C type model and not making assumptions about
type sizes, so most places are fine.

The places I can think of easily that have issues are
AST::getSizeType() and friends, and HandleModeAttr in
SemaDeclAttr.cpp, although it's possible there are others.

Targets really need to have a way to explicitly define the types of
intmax_t, size_t, ptrdiff_t, and wchar_t. In addition to it being
messy to derive these types from the widths of the integer/pointer
types, it's actually impossible: for example, size_t should be
"unsigned int" on Linux and "unsigned long" on OSX. Here's a quick
proposal: we add an enum IntType {UnsignedShort, SignedShort,
UnsignedInt, SignedInt, UnsignedLong, SignedLong, UnsignedLongLong,
SignedLongLong} to TargetInfo. We add 4 new methods to TargetInfo,
SizeType, IntMaxType, PtrDiffType, and WCharType, returning values
from the enum. The preprocessor and ASTContext then key off of these
methods for the relevant types rather than trying to guess.

For HandleModeAttr, I'm not really sure what gcc does; could someone
check the output of the following on OSX 32-bit?

typedef long a __attribute__((__mode__(__SI__)));
int x[__builtin_types_compatible_p(a, long) ? 1 : -1];

-Eli

mrs $ gcc -c -arch ppc t.c
t.c:2: error: size of array ‘x’ is negative
mrs $ gcc -c -arch i386 t.c
t.c:2: error: size of array ‘x’ is negative
mrs $ gcc -c -arch x86_64 t.c
t.c:2: error: size of array ‘x’ is negative
mrs $ gcc -c -arch ppc64 t.c
t.c:2: error: size of array ‘x’ is negative

Thanks for the information,
For now in PIC16 port, I'll be setting the size of long long to zero and
by checking that in Preprocessor.cpp, I know if long long is supported
or not. Tentatively in the place where it is asserting, I'll leaving the
current code only for targets that have non zero long long.
I don't know if you want me to submit this patch or not. It seems to be
working for us at least for now until the final approach is adopted.

Regarding your proposal,
Similar enum is defined publicly in BuiltinType class. Can't that be
used instead of creating a new enum to become source of confusion?

Eli wrote:

There is an assertion in the Preprocessor.cpp line 600 that assumes
current targets have sizeof(long) == sizeof(void *). This assumption is
incorrect.

Yeah; the only reason Chris did that was to avoid adding a few extra
cases to the code. Feel free to submit a patch, or else I'm sure
Chris will get to it soon.

Right. I just didn't know the logic to infer the preprocessor definitions, it should be easy to add that code.

I agree with your type proposal.

Thanks for the information,
For now in PIC16 port, I'll be setting the size of long long to zero and
by checking that in Preprocessor.cpp, I know if long long is supported
or not. Tentatively in the place where it is asserting, I'll leaving the
current code only for targets that have non zero long long.
I don't know if you want me to submit this patch or not. It seems to be
working for us at least for now until the final approach is adopted.

Why not just make sizeof(long long) == sizeof(long)? It would be nicer to support long long, even if it is the same size as long.

Regarding your proposal,
Similar enum is defined publicly in BuiltinType class. Can't that be
used instead of creating a new enum to become source of confusion?

BuiltinType is part of the AST library. TargetInfo is in the "Basic" library. Code in 'Basic' can't use enums in AST. Please add a new enum to TargetInfo.

Thanks!

-Chris

I applied the modifications locally as suggested by Eli.
Will check on our port and we will submit the patch.

A.

From: Chris Lattner [mailto:clattner@apple.com]
Sent: Tuesday, October 21, 2008 9:40 PM
To: Alireza Moshtaghi - C13012
Cc: cfe-dev@cs.uiuc.edu
Subject: Re: [cfe-dev] size of pointer and size of long

Eli wrote:
>> There is an assertion in the Preprocessor.cpp line 600 that assumes
>> current targets have sizeof(long) == sizeof(void *). This
>> assumption is
>> incorrect.
>
> Yeah; the only reason Chris did that was to avoid adding a few extra
> cases to the code. Feel free to submit a patch, or else I'm sure
> Chris will get to it soon.

Right. I just didn't know the logic to infer the preprocessor
definitions, it should be easy to add that code.

I agree with your type proposal.

> Thanks for the information,
> For now in PIC16 port, I'll be setting the size of long long to zero
> and
> by checking that in Preprocessor.cpp, I know if long long is

supported