Ping^2: [patch] convert some call ABI macros to hooks, apply to sh, add bitfield swapper

DJ Delorie dj@redhat.com
Sat Aug 30 00:08:00 GMT 2003


> + 	fprintf(stderr, "\n-- aggregate_value_p --\n");
> + 	debug_tree (fntype);
> + 	break;
> 
> Left some debugging in.

Oops.  Actually, it's not quite debugging.  Perhaps it should be an
abort, since I used that to find all the types of callers.

> -    Copyright (C) 1989, 1992, 1993, 1994, 1995, 1996, 1997, 1998
> +    Copyright (C) 1989, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 2003
>      1999, 2000, 2001, 2002, 2003 Free Software Foundation, Inc.
> 
> Emacs copyright brokenness.

Sigh, this is the third one emacs has messed up.  I just upgraded it,
maybe that will help.

> +   if ((* targetm.ms_bitfield_layout_p) (rli->t)
> +       && (* targetm.reverse_bitfield_layout_p) (rli->t)
> +       && ! TYPE_PACKED (rli->t))
> +       reverse_bitfield_layout (rli);
> 
> Why would ms_bitfield_layout_p or TYPE_PACKED have anything
> necessarily to do with reverse_bitfield_layout_p?  Seems like
> these tests, if needed, should be pushed down into the 
> reverse_bitfield_layout_p predicate.

Consider this case:

	struct {
	  int i:4
	  char j:4
	} foo;

The layout normally goes like this (little endian):

	i = bits 0..3 of SI at byte offset 0
	j = bits 4..7 of QI at byte offset 0

Now, asking gcc to put the bits at the other end of "the" underlying
type becomes nonsensical, because there is no *one* type underlying.
In order to even consider the reversal, there must be just one
underlying type for each grouping of bitfields, and the conditionals I
specified (in theory) do that.  IMHO gcc cannot honor a request to
reverse them (reverse_bitfield_layout_p) unless those conditions are
met, so there's no point letting the target decide.

The reason such bitpacking works with gcc anyway is because gcc always
packs from the struct's byte 0, regardless of endianness, so the order
of bit allocation is independent of the underlying types.

So, while it's possible to push those tests to the target, I don't
think it's appropriate to do so.



More information about the Gcc-patches mailing list