Patch for IA64 32-bit function descriptors

[Richard Henderson]

On Fri, May 31, 2002 at 02:35:51PM -0700, Jessica Han wrote:
> I submitted the following patch a while ago, it hasn't been checked in yet.
> Could somebody review it and check it in if that's okay? Thanks.

Is this any different from the last time I reviewed it?

> + /* vtable entries are 8 byte aligned at 32-bit mode */
> + #undef TARGET_VTABLE_ENTRY_ALIGN
> + #define TARGET_VTABLE_ENTRY_ALIGN ((TARGET_ILP32)? 8:0)

Should be the alignment in bits, always.  I.e. the constant 64.

> + /* By default, the vtable entries are void pointers. Setting this non-zero
> +    tells the compiler to have special aligned vtable.  The value of
> +    this macro says how many byte the entries should be aligned. */
> + #ifndef TARGET_VTABLE_ENTRY_ALIGN
> + #define TARGET_VTABLE_ENTRY_ALIGN 0

Again, the alignment in bits, always.  I.e. POINTER_SIZE.

> +   /* HP linker requires vtable info 8-byte aligned on both 32-bit mode and
> +      64 bit mode */

Comment doesn't belong here.

> +   DECL_ALIGN(vtbl) = MAX(TYPE_ALIGN (double_type_node),DECL_ALIGN(vtbl));

double_type_node?  Why do you think you introduced
TARGET_VTABLE_ENTRY_ALIGN?

Space before open paren.  Probably don't need the MAX either.

> !   /* The first vbase or vcall offset is at index -3 in the vtable at 64
> bit
> !      mode and -6 at 32-bit mode because of the padding.  */
> !   vid.index = ssize_int (TARGET_VTABLE_ENTRY_ALIGN ? -6 : -3);

No.  You *cannot* do magic constants.  I cannot stress this
strongly enough.

And surely you don't need anything here, since your vtable entry
size is 4, so you should already have your multiplier.

And "64-bit mode" and "32-bit mode" are definitely out of context
here.  ia32 is "32-bit mode" and doesn't require such changes.

> !   /* HP Linker requires that vtable info 8-byte aligned at both 32-bit and
> !      64 bit mode. We put a 4-byte zero after each entry of the vtable to
> !      make gcc generate aCC like vtables at 32-bit, thus HP linker can
> !      recognize them properly.
> !    */
> !   if (TARGET_VTABLE_ENTRY_ALIGN)

What is this?  Why isn't the existing code to put 
N=TARGET_VTABLE_USES_DESCRIPTORS-1 zeros sufficient?

> !       /* The next vbase will come at a more negative offset.  1 vtable
> entry
> !          at 64-bit mode, 2 vtable entries at 32-bit mode */
> !       vid->index = size_binop (MINUS_EXPR, vid->index, ssize_int
> (TARGET_VTABLE_ENTRY_ALIGN ? 2:1));

Again, no.  Your vtable entry size is 4.  That should be sufficient.

>         /* The next vcall offset will be found at a more negative
> ! 	 offset, 1 vtable entry at 64-bit mode, 2 vtable entries at 32-bit mode
> */
> !       vid->index = size_binop (MINUS_EXPR, vid->index,
> !                                ssize_int (TARGET_VTABLE_ENTRY_ALIGN ? 2 :

Nope.

> !   /* The offset-to-top field is at index -2 from the vptr at 64-bit mode,
> !      -4 at 32-bit mode.  */
> !   index = build_int_2 ((TARGET_VTABLE_ENTRY_ALIGN ? -4 : -2), -1);

Nope.

> !       /* The RTTI information is at index -1 at 64-bit mode, -2 at 32-bit
> mode.  */
> !       index = (TARGET_VTABLE_ENTRY_ALIGN? build_int_2 (-2, -1) :
> integer_minus_one_node);

Nope.

> ! 				   size_int (TARGET_VTABLE_ENTRY_ALIGN ? 4 : 2),

Nope.



r~