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] adjust make_range


On Fri, 2004-06-11 at 05:13, Eric Christopher wrote:
> > Sorry, but you're going to have to dig deeper.
> 
> *digs deeper*
> 
> OK. I think I found it this time :)
> 
> It's late though so I'm going to test this, but I think in this code
> here:
> 
> if (TREE_CODE_CLASS (code) == '<'
>     || TREE_CODE_CLASS (code) == '1'
>     || TREE_CODE_CLASS (code) == '2')
>   type = TREE_TYPE (arg0);
> 
> we actually want the type of the expression (exp) so that we can merge
> ranges when we have expressions on the lhs of exp, e.g.
> 
> (cond - 2) >= 100
> 
> so we'd change that to:
> 
> type = TREE_TYPE (exp);
Hmmm, what it appears you're suggesting is that we use the type of
the expression rather than the type of the argument throughout that
code.

Unfortunately, this code doesn't really describe how "type" is used,
so it is exceedingly difficult to know if this is a safe and correct
change.  However, my gut tells me the change is wrong.

Let's look at the code for the comparison operators -- that code peeks
at TYPE_UNSIGNED (type).  I'm pretty sure we wanted to refer to the
type of the argument in that context, not the type of the expression
(think about it -- the result type of these expression is a boolean).

The other operators where the type of the expression is likely to
differ materially from the type of the first operand are NOP_EXPR
and CONVERT_EXPR.


Even within the code for handling NOP_EXPR and CONVERT_EXPR I
think such a change is wrong.  Consider this statement:

          if (TYPE_UNSIGNED (type) && ! TYPE_UNSIGNED (TREE_TYPE (exp)))


It seems to me with your change that will always be false since
type == TREE_TYPE (exp)



Rather than continue to propose patches, why don't you describe in
more detail the ranges at the various stages in this routine.  ie,
I'm pretty sure you go around the loop in that routine 2-3 times
for the ((int)cond - 2) > 100 expression.

Note the tree you are processing each iteration of the loop and
the range (in_p, low, high).  Also note that range you think ought
to have been computed for each iteration where it differs.

That might help us decipher what's happening vs what ought to be
happening.

jeff



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