This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


> + 	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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]