Re-visiting this. If we have source like this:
extern void func();
long long fptr = (long long)&func;
and pointers are smaller than "long long" (example: i386 or m32c), the
compiler will first convert the pointer to an int, then convert the
int to a long long. This results in output like this:
.longfunc
.long0
Which isn't useful if the address happens to be bigger than int
(example: reset vectors on m32c), and is just plain wrong on big
endian machines.
I've figured out a few changes that let this work in a useful way, but
some of the assumptions may not be globally valid, so I'm asking for
confirmation here.
First off, in initializer_constant_valid_p, we don't assume there's
only one conversion. If we see a NP_EXPR or CNVERT_EXPR, we peel of
however many there are. We only really care about the inner and outer
types at this point, as no more compile-time conversions happen after
this point (true?).
Second, in output_constant, we add an assumption that constants which
are the address of some DECL can be extended by the assembler in a
meaningful way. Certainly, gcc isn't doing a good job of extending
them, and if such an extension isn't possible, it's already too late
for gcc to do it at compile time, might as well let the assembler or
linker emit the diagnostic (gcc's current diagnostic is misleading).
Lastly, m32c knows how to widen pointers in the assembler. (ok, I
happen to know this assumption is valid ;)
Index: varasm.c
RCS file: /cvs/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.525
diff -p -U3 -r1.525 varasm.c
varasm.c17 Aug 2005 19:25:12 -00001.525
varasm.c19 Aug 2005 03:34:58 -0000
@@ -3652,9 +3652,13 @@ initializer_constant_valid_p (tree value
tree src_type;
tree dest_type;
-src = TREEPERAND (value, 0);
-src_type = TREE_TYPE (src);
dest_type = TREE_TYPE (value);
+src = value;
+do {
+ src = TREEPERAND (src, 0);
+} while (TREE_CDE (src) == CNVERT_EXPR
+ || TREE_CDE (src) == NP_EXPR);
+src_type = TREE_TYPE (src);
/* Allow conversions between pointer types, floating-point
types, and offset types. */
@@ -3857,6 +3861,13 @@ output_constant (tree exp, unsigned HST
code = TREE_CDE (TREE_TYPE (exp));
thissize = int_size_in_bytes (TREE_TYPE (exp));
+ /* Assume addresses of decls can be extended to larger types by the
+ assembler/linker, or at least, that they'll fail otherwise. */
+ if (TREE_CDE (exp) == ADDR_EXPR
+ && DECL_P (TREEPERAND (exp, 0))
+ && thissize < size)
+ thissize = size;
+
/* Allow a constructor with no elements for any data type.
This means to fill the space with zeros. */
if (TREE_CDE (exp) == CNSTRUCTR
Index: config/m32c/m32c.c
RCS file: /,v
retrieving revision 1.3
diff -p -U3 -r1.3 m32c.c
config/m32c/m32c.c16 Aug 2005 00:31:39 -00001.3
config/m32c/m32c.c19 Aug 2005 03:34:59 -0000
@@ -1972,6 +1972,15 @@ m32c_asm_integer (rtx x, unsigned int si
output_addr_const (asm_out_file, x);
fputc ('\n', asm_out_file);
return true;
+ case 4:
+ if (GET_CDE (x) == SYMBL_REF)
+{
+ fprintf (asm_out_file, "\t.long\t");
+ output_addr_const (asm_out_file, x);
+ fputc ('\n', asm_out_file);
+ return true;
+}
+ break;
}
return default_assemble_integer (x, size, aligned_p);
}

Thu, Aug 18, 2005 at 11:48:26PM -0400, DJ Delorie wrote:
-src = TREEPERAND (value, 0);
-src_type = TREE_TYPE (src);
dest_type = TREE_TYPE (value);
+src = value;
+do {
+ src = TREEPERAND (src, 0);
+} while (TREE_CDE (src) == CNVERT_EXPR
+ || TREE_CDE (src) == NP_EXPR);
+src_type = TREE_TYPE (src);

/* Allow conversions between pointer types, floating-point
types, and offset types. */

I'd prefer if we validated the types at each step, and not just
the inner and outer types. So (int)(float)(int)&f fails.

Does the problem appear in practice because TARGET_VALID_PINTER_MDE
is not defined for m32c? I was thinking that maybe if the expansion
of the expr produced

(symbol_ref:SI "foo")
instead of
(symbol_ref:HI "foo")

things might work.

I'm slightly leery of

+ /* Assume addresses of decls can be extended to larger types by the
+ assembler/linker, or at least, that they'll fail otherwise. */
+ if (TREE_CDE (exp) == ADDR_EXPR
+ && DECL_P (TREEPERAND (exp, 0))
+ && thissize < size)
+ thissize = size;

this otherwise.

r~

Does the problem appear in practice because TARGET_VALID_PINTER_MDE
is not defined for m32c?
That's only used for __attribute__((mode)) on pointer variables, which
doesn't help this situation.

I'd prefer if we validated the types at each step, and not just
the inner and outer types. So (int)(float)(int)&f fails.

What kind of validation is valid? The case I'm trying to fix is essentially:

(long)(short)&f

In this case, I need to drop the (short) and cast directly from the
address to long, since on m16c physical addresses are 20 bits but
"pointers" are only 16 bits, so it's legal to drop the (short) cast.

Tue, Aug 23, 2005 at 04:28:54PM -0400, DJ Delorie wrote:
That's only used for __attribute__((mode)) on pointer variables, which
doesn't help this situation.
So why aren't you getting (symbol_ref:SI "foo")?
r~

Tue, Aug 23, 2005 at 04:32:32PM -0400, DJ Delorie wrote:
What kind of validation is valid? The case I'm trying to fix is essentially:

(long)(short)&f

In this case, I need to drop the (short) and cast directly from the
address to long, since on m16c physical addresses are 20 bits but
"pointers" are only 16 bits, so it's legal to drop the (short) cast.

Well, no. If the result isn't the sign-extended version of the
short, then you got the wrong answer, regardless of how many bits
are in m16c addresses.

What you're trying to generate is something that has all 20 bits
valid. am I mistaking what ".long f" will do?

r~

Well, no. If the result isn't the sign-extended version of the
short, then you got the wrong answer, regardless of how many bits
are in m16c addresses.

Good point.

In this case (long x = (long)&f) IMH the intermediate step shouldn't
have been added, because it implies a conversion that wasn't asked for
- conversion to "int". I've seen a lot of cases where things are
silently converted to int before being converted to the desired mode,
and while some are understandable, it does get annoying at times.

What you're trying to generate is something that has all 20 bits
valid. am I mistaking what ".long f" will do?

The correct thing to do is emit ".long f".

Addresses on the r8c/m16c/m32c family are, well, interesting. The r8c
and m16c have a 20 bit physical address bus, but only 16 bit address
registers. You can concatenate two registers to form a 32 bit
pointer, or use a 24 bit displacement off a 16 bit register (er, a 24
bit base address plus a 16 bit offset). "int" is 16 bits. The reset
vector (well, any interrupt vector) is 32 bits, but only the lower 24
bits are used, and they hold a 20 bit address. The upper 8 bits, fyi,
hold portions of the flash security key. Most people set the key to
all zeros just to simplify things. For gcc, .text goes above 64k but
everything else stays below it.

The m32c has a 24 bit physical address bus, and 24 bit pointers,
and get this 24 bit address registers. Pointers here are
PSImode and convert to 32 bit scalars just fine. In fact, PSImode
outside of the address registers are 32 bits anyway. Because the
general registers are 16 bits, though, size_t (and thus array index
math) is done in 16 bits, limiting the size of individual items,
although such items can exist above the 64k address.

The assembler and linker treat everything like 32 bit addresses and
just truncate them according to where they're going. If you have to
fit a function pointer into 16 bits and it's outside the first 64k, a
thunk is introduced - however, this doesn't work for reset vectors
because they're needed long before the thunks can be copied to low
memory.

So why aren't you getting (symbol_ref:SI "foo")?
Pmode is HImode. See other email.

Tue, Aug 23, 2005 at 10:44:57PM -0400, DJ Delorie wrote:
So why aren't you getting (symbol_ref:SI "foo")?

Pmode is HImode. See other email.

Nevertheless, we see symbol_ref in both SImode and DImode when
doing mixed pointer size stuff on MIPS and s390x. So pleased
to be investigating why it isn't happening for you.

r~

Tue, Aug 23, 2005 at 10:44:15PM -0400, DJ Delorie wrote:
The correct thing to do is emit ".long f".

Correction: ".long f" is what you want to generate. It's not
correct for (long)(short)&f.

doesn't sound like it; I'll note that while you gave an
synopsis of the processor family, you didn't actually answer
the specific question about the assembler and the relocation
that it will emit.

r~

Correction: ".long f" is what you want to generate. It's not
correct for (long)(short)&f.

The test program only has (long)&f in the C source. The (short) isn't
something I added myself; it's something gcc adds for me, which I
infer from the tree we end up with.

doesn't sound like it; I'll note that while you gave an
synopsis of the processor family, you didn't actually answer
the specific question about the assembler and the relocation
that it will emit.

m32c-elf does have a 32-bit relocation for symbol addresses. If GCC
gives the assembler ".long f" the assembler will emit an R_M32C_32
relocation (via BFD_RELC_32) against f. If I hand-code this in
assembler, it works fine. The assembler can also handle ".short f"
with R_M32C_16 (via BFD_RELC_16) and ".3byte f" with R_M32C_24 (via
BFD_RELC_24). There's also a couple of hi/lo and pcrel relocs that
aren't relevent to this problem, and that's pretty much it for
m32c-elf relocs.

Wed, Aug 24, 2005 at 01:17:50AM -0400, DJ Delorie wrote:
Correction: ".long f" is what you want to generate. It's not
correct for (long)(short)&f.

The test program only has (long)&f in the C source. The (short) isn't
something I added myself; it's something gcc adds for me, which I
infer from the tree we end up with.

Hmm. How about

typedef void * lptr __attribute__((mode(SI)));
long l = (long)(lptr)&l;

?

r~

typedef void * lptr __attribute__((mode(SI)));
long l = (long)(lptr)&l;

(with the TARGET_VALID_PINTER_MDE change)

typedef int *longpointer __attribute__((mode(SI)));
extern void reset();

longpointer j = (longpointer)&reset;

.file"dj.c"
.global_j
data
.type_j, @object
.size_j, 4
_j:
.short_reset
.zero2
.ident"GCC: (GNU) 4.1.0 20050819 (experimental)"

Nevertheless, we see symbol_ref in both SImode and DImode when
doing mixed pointer size stuff on MIPS and s390x. So pleased
to be investigating why it isn't happening for you.

, I found something useful. In initializer_constant_valid_p(), and
later in output_constant(), we're presented with this:

<nop_expr
type <pointer_type
type <function_type QI>
unsigned SI[*]
>
arg 0 <addr_expr
type <pointer_type HI>
arg 0 <function_decl>
>
>
[*] this is from the __attribute__((mode(SI))) I assume

However, in both functions we blindly call TREE_TYPE (TREEPERAND
(exp, 0)) which ignores the "SI" override in the type we're converting
to.

output_constant has this comment:

/* Eliminate any conversions since we'll be outputting the underlying
constant. */

but we don't want to eliminate the conversion, because that
discards the size change! We end up emitting a truncated initializer,
then padding with zeros to fill the needed size.

Ideas?

dj.c
typedef void (*longpointer)() __attribute__((mode(SI)));
extern void reset();

longpointer j = (longpointer)&reset;

Nevertheless, we see symbol_ref in both SImode and DImode when
doing mixed pointer size stuff on MIPS and s390x.

FYI I can reproduce this with mips-elf too.

$ ./cc1 -quiet dj.c
dj.c:3: error: initializer element is not constant

dj@greed pts/0 ~/gnu/gcc/mips-elf/gcc
$ cat dj.c
extern void reset();

long long j = (long long)&reset;

dj@greed pts/0 ~/gnu/gcc/mips-elf/gcc
$ cat dj.s
.file 1 "dj.c"
.section .mdebug.abi32
.previous
.globl j
.section .sbss,"aw",@nobits
.align 3
.type j, @object
.size j, 8
j:
.space 8
.ident "GCC: (GNU) 4.1.0 20050826 (experimental)"

The assumption in output_constant is that you've emitted something like
".word" or ".8byte" that says how wide the value is. Then, you just
emit the value.

Do you have a way in your assembler to emit a 32-bit pointer? If not,
then you have to declare this an invalid initialization. If you do have
such a way, then you have to set up things in the code that calls
output_constant to say "the value you see next is a 32-bit pointer".

Yes, I have all that, and if I hack varasm to note the right type, it
emits exactly what I want - ".long reset". Check out the earlier
posts in this thread; rth and I have gone back and forth a lot for
this one.

And assemble_variable() doesn't emit the .long anyway,
output_constant() does.

I may not be being helpful, but why can't you handle this in
assemble_integer? You should get the size (4) and an appropriate
RTX, if I'm reading correctly.

I *do* handle it (local sources). I'm not given the right size, I
checked. And the symbol_ref is HImode.

DJ Delorie <dj (AT) redhat (DOT) comwrites:
FYI I can reproduce this with mips-elf too.

$ ./cc1 -quiet dj.c
dj.c:3: error: initializer element is not constant

dj@greed pts/0 ~/gnu/gcc/mips-elf/gcc
$ cat dj.c
extern void reset();

long long j = (long long)&reset;

I might be missing the point, but the error is correct for o32.
There's no official o32 relocation that can be used to create
a properly sign-extended value.

EABI does provide R_MIPS_64 (not an official o32 reloc), and I suppose
you could argue that that reloc makes the test valid for the 32-bit EABI.
I'm not sure it needs to be valid even there though. It seems odd to
support the construct in only one of the many MIPS cases where:

sizeof (long long) != sizeof (void *)

Personally, I'd be happy to say "not supported" for all such cases.
I'd certainly prefer having an error for 32-bit EABI over not having
an error for the cases where it isn't valid.

Richard

X-Sieve: CMU Sieve 2.2
From: Richard Sandiford <richard (AT) codesourcery (DOT) com>
Mail-Followup-To: DJ Delorie <dj (AT) redhat (DOT) com>,rth (AT) redhat (DOT) com, gcc-patches (AT) gcc (DOT) gnu.org, richard (AT) codesourcery (DOT) com
Cc: rth (AT) redhat (DOT) com, gcc-patches (AT) gcc (DOT) gnu.org
Date: Wed, 31 Aug 2005 10:54:58 +0100
X-RedHat-Spam-Score: 0

DJ Delorie <dj (AT) redhat (DOT) comwrites:
FYI I can reproduce this with mips-elf too.

$ ./cc1 -quiet dj.c
dj.c:3: error: initializer element is not constant

dj@greed pts/0 ~/gnu/gcc/mips-elf/gcc
$ cat dj.c
extern void reset();

long long j = (long long)&reset;

I might be missing the point, but the error is correct for o32.

The point is twofold:

1. MIPS is going through the same logic that produces an *invalid*
initializer for other chips, even considering the lack of support.

2. The M32C *does* have full support for this in the chip, but gcc
doesn't.

Imagine, if you will, a MIPS chip with 64 bit address space but 32 bit
registers. Most of the time, 32 bit pointers are ideal. Sometimes
you need the full 64 bit address of something, such as a reset vector
(which is the specific case I need the m32c fix for). GCC has no way
of initializing a 64 bit variable with the 64 bit address of
something, when pointers don't default to 64 bits, even if the
assembler and linker support it. It's not a question of whether the
target supports it or not; gcc sees a conversion that looks like
address->int->long and says "I can't do that". course it can't,
but the target and/or assembler might be able to, because they know
if/how addresses get extended or at least referred to in an extended
form.

The actual code I'm trying to make work for m32c looks something like
this:

void my_reset(void);
long reset_vector __attribute__((section(".resetvec"))) = (long)&my_reset;

The assembler and linker support this fully. The chip has the address
space and relocs for this, even though the default pointer size is 16
bits (GPRs are 16 bits). I can't get gcc to support it.

Consider the i386's 16-bit "medium model". All the data fits inside
one 64k segment, and pointers are 16 bits. But functions live
elsewhere in the 1mb address map, and *function* pointers are bigger.

The xstormy16 chip is similar too. For xstormy16 and m32c, if you
have a 16 bit reloc for a function outside the 64k address space, a
code stub is created by the linker inside the 64k code space, which
just jumps to the real 32-bit address. This works for everything
*except* reset vectors.

I'd prefer if we validated the types at each step, and not just
the inner and outer types. So (int)(float)(int)&f fails.

How does the attached patch look? It requires a slightly different
test case (i.e. "more legal" ;) but allows a more precise validation
of the multiple conversions.

The second chunk of the patch catches the cases where we're putting a
declarator in a too-small block of memory. I'm pretty sure we need to
limit this check to scalar types, as they're the ones that suffer from
the endianness problem. I leave it in for the moment as a point of
discussion; I think we should detect when scalar conversions are
erroneously stripped.

New test case:

typedef void (*longpointer)() __attribute__((mode(SI)));
extern void reset();
longpointer lp = (longpointer)&reset;

Does the problem appear in practice because TARGET_VALID_PINTER_MDE
is not defined for m32c? I was thinking that maybe if the expansion
of the expr produced

I added that, but it didn't produce it. I committed support for that
anyway, as it's needed to prototype 32-bit pointers. It turns out
that doing that, and using the longpointer prototype above, *almost*
lets you put data in far memory. The optimizer is still making
assumptions about pointer sizes somewhere, I might poke at it a little
and see if there are any easy wins in that area. It would be nice if
we could at least selectively put const data in far memory, as for
m16c (the variant where it matters most) there's precious little RAM
and lots of flash, but the flash is all outside the first 64k.

Index: varasm.c

RCS file: /cvs/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.525
diff -p -U3 -r1.525 varasm.c
varasm.c17 Aug 2005 19:25:12 -00001.525
varasm.c1 Sep 2005 01:37:12 -0000
@@ -3847,12 +3847,50 @@ output_constant (tree exp, unsigned HST
if (size == 0 || flag_syntax_only)
return;

+ /* See if we're trying to intialize a pointer in a non-default mode
+ to the address of some declaration somewhere. If the target says
+ the mode is valid for pointers, assume the target has a way of
+ resolving it. */
+ if (TREE_CDE (exp) == NP_EXPR
+ && TREE_CDE (TREE_TYPE (exp)) == PINTER_TYPE
+ && targetm.valid_pointer_mode (TYPE_MDE (TREE_TYPE (exp))))
+ {
+ tree op = exp;
+ tree saved_type = TREE_TYPE (exp);
+
+ /* Peel off any intermediate conversions-to-pointer for valid
+ pointer modes. */
+ while (TREE_CDE (exp) == NP_EXPR
+ && TREE_CDE (TREE_TYPE (exp)) == PINTER_TYPE
+ && targetm.valid_pointer_mode (TYPE_MDE (TREE_TYPE (exp))))
+exp = TREEPERAND (exp, 0);
+
+ /* If what we're left with is the address of something, we can
+ convert the address to the final type and output it that
+ way. */
+ if (TREE_CDE (exp) == ADDR_EXPR)
+TREE_TYPE (exp) = saved_type;
+ }
+
/* Eliminate any conversions since we'll be outputting the underlying
constant. */
while (TREE_CDE (exp) == NP_EXPR || TREE_CDE (exp) == CNVERT_EXPR
|| TREE_CDE (exp) == NN_LVALUE_EXPR
|| TREE_CDE (exp) == VIEW_CNVERT_EXPR)
- exp = TREEPERAND (exp, 0);
+ {
+ HST_WIDE_INT type_size = int_size_in_bytes (TREE_TYPE (exp));
+ HST_WIDE_INT op_size = int_size_in_bytes (TREE_TYPE (TREEPERAND (exp, 0)));
+
+ /* Make sure eliminating the conversion is really a no-op. */
+ if (type_size != op_size)
+{
+ error ("no-op convert from %d bytes to %d bytes in initializer",
+ op_size, type_size);
+ gcc_unreachable ();
+}
+
+ exp = TREEPERAND (exp, 0);
+ }

code = TREE_CDE (TREE_TYPE (exp));
thissize = int_size_in_bytes (TREE_TYPE (exp));

Wed, Aug 31, 2005 at 10:03:11PM -0400, DJ Delorie wrote:
How does the attached patch look? It requires a slightly different
test case (i.e. "more legal" ;) but allows a more precise validation
of the multiple conversions.

Better. The basic idea I can accept.

+ && TREE_CDE (TREE_TYPE (exp)) == PINTER_TYPE

PINTER_TYPE_P.

+TREE_TYPE (exp) = saved_type;

I'm not fond of destructively modifying the expression. You can
create a new ADDR_EXPR here just find though.

+ error ("no-op convert from %d bytes to %d bytes in initializer",
+ op_size, type_size);
+ gcc_unreachable ();

internal_error.

r~

Better. The basic idea I can accept.

Updated patch. No regressions on x86-64, but I'm still leery of the
internal_error being inappropriate for all cases. Maybe I'm missing a
key assertion here; but the padding is specifically for types that
don't completely fill their slots (incomplete structures, partial
arrays). Could *any* of that sort of thing have a conversion on it?

* varasm.c (output_constant): Let the target resolve
conversions of addresses to non-default pointer sizes.

Index: varasm.c

RCS file: /cvs/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.525
diff -p -U3 -r1.525 varasm.c
varasm.c17 Aug 2005 19:25:12 -00001.525
varasm.c1 Sep 2005 22:51:36 -0000
@@ -3847,12 +3847,46 @@ output_constant (tree exp, unsigned HST
if (size == 0 || flag_syntax_only)
return;

+ /* See if we're trying to intialize a pointer in a non-default mode
+ to the address of some declaration somewhere. If the target says
+ the mode is valid for pointers, assume the target has a way of
+ resolving it. */
+ if (TREE_CDE (exp) == NP_EXPR
+ && PINTER_TYPE_P (TREE_TYPE (exp))
+ && targetm.valid_pointer_mode (TYPE_MDE (TREE_TYPE (exp))))
+ {
+ tree saved_type = TREE_TYPE (exp);
+
+ /* Peel off any intermediate conversions-to-pointer for valid
+ pointer modes. */
+ while (TREE_CDE (exp) == NP_EXPR
+ && PINTER_TYPE_P (TREE_TYPE (exp))
+ && targetm.valid_pointer_mode (TYPE_MDE (TREE_TYPE (exp))))
+exp = TREEPERAND (exp, 0);
+
+ /* If what we're left with is the address of something, we can
+ convert the address to the final type and output it that
+ way. */
+ if (TREE_CDE (exp) == ADDR_EXPR)
+exp = build1 (ADDR_EXPR, saved_type, TREEPERAND (exp, 0));
+ }
+
/* Eliminate any conversions since we'll be outputting the underlying
constant. */
while (TREE_CDE (exp) == NP_EXPR || TREE_CDE (exp) == CNVERT_EXPR
|| TREE_CDE (exp) == NN_LVALUE_EXPR
|| TREE_CDE (exp) == VIEW_CNVERT_EXPR)
- exp = TREEPERAND (exp, 0);
+ {
+ HST_WIDE_INT type_size = int_size_in_bytes (TREE_TYPE (exp));
+ HST_WIDE_INT op_size = int_size_in_bytes (TREE_TYPE (TREEPERAND (exp, 0)));
+
+ /* Make sure eliminating the conversion is really a no-op. */
+ if (type_size != op_size)
+internal_error ("no-op convert from %d to %d bytes in initializer",
+(int)op_size, (int)type_size);
+
+ exp = TREEPERAND (exp, 0);
+ }

code = TREE_CDE (TREE_TYPE (exp));
thissize = int_size_in_bytes (TREE_TYPE (exp));

Thu, Sep 01, 2005 at 06:52:46PM -0400, DJ Delorie wrote:
Could *any* of that sort of thing have a conversion on it?

I wouldn't think so. I guess we'll find out.

* varasm.c (output_constant): Let the target resolve
conversions of addresses to non-default pointer sizes.

except,

+internal_error ("no-op convert from %d to %d bytes in initializer",
+(int)op_size, (int)type_size);

%wd should print HST_WIDE_INT values properly.

r~

+internal_error ("no-op convert from %d to %d bytes in initializer",
+(int)op_size, (int)type_size);
%wd should print HST_WIDE_INT values properly.
, I didn't know what to do with those.
Committed with that change.
Thanks!