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: [PATCH] Handle BOOLEAN_TYPEs in c_common_type


Hi Richard and Joseph,

Thanks for your speedy comments.

On Mon, 16 May 2005, Joseph S. Myers wrote:
> On Sun, 15 May 2005, Roger Sayle wrote:
> > 2005-05-15  Roger Sayle  <roger@eyesopen.com>
> >
> > 	* c-typeck.c (c_common_type): Also handle BOOLEAN_TYPEs.
>
> The specification of c_common_type as expressed in the comment above it is
> that the default conversions have already been applied, so boolean types
> shouldn't be seen there.  Maybe you should put the special cases in the
> common_type wrapper and adjust its specification accordingly?

With my local changes to the tree, the failing testcase is
gcc.c-torture/execute/20030714-1.c.  The sequence of events is that
build_binary_op attempts to construct a NE_EXPR operation on two
QImode Boolean values that have been promoted to SImode integers
following the usual conversion rules.  Line 7857 of build_binary_op
calls shorten_compare on the integer operands, and then in
c-common.c:shorten_compare, we call get_narrower to reduce the
promoted arguments to their unwidened forms.  In this case, although
op0 and op1 are integer types, the results of get_narrower, primop0
and primop1 are now Boolean types.

Finally, on line 2158 of shorten_compare, we end up calling
type = common_type (TREE_TYPE (primop0), TREE_TYPE (primop1));

And by passing two unpromoted Booleans to c_common_type, we ICE.


The possible solutions are (i) that get_narrower shouldn't strip
the Boolean to integer conversions, (ii) shorten_compare should
guard against BOOLEAN_TYPE before calling common_type, (iii)
that common_type and not c_common_type should handle Booleans
(as proposed by Joseph) or (iv) my original patch that extends
the functionality of c_common_type.

I completely agree that as documented in the commentary this
shouldn't ever happen.  But given the sequence of events above,
the problem becomes how and where to prevent this case slipping
through.

I've also checked that the original build_binary_op is being
called directly from the C parser via parser_build_binary_op,
so this isn't a problematic middle-end or tree-ssa transformation
that's leaking to build_binary_op later in processing (as may
have been possible with my previous latent bug fix).


Any suggestions on where this should correctly be fixed?
Strength reducing the gcc_assert and adding support to a routine
that hadn't previously encounter this situation seemed like a
safe approach.  It also had the benefit of allowing the compiler
to generate a QImode comparison.

Roger
--


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