[ Another patch I'd started working through, but hadn't completed... ]
On 09/14/2016 07:05 AM, Richard Biener wrote:
On Mon, Aug 22, 2016 at 4:58 PM, Robin Dapp <rd...@linux.vnet.ibm.com> wrote:
if !inner_ovf (just set that to false if !check_inner_ovf to simplify
checks please).
you know it's valid to transform the op to (T)@0 innerop (T)@1 outerop @2
(if T is wider than the inner type which I think you should check and
which should
simplify things).
I simplified the control flow a little and split both parts of the
combination into separate patterns.
You can then combine (T)@1 and @2 where I think you fail to handle mixed
MINUS_EXPR/PLUS_EXPR (practically you'll see only PLUS_EXPRs for
integers).
Is it ok to do something like this (see patch) in order to deal with
MINUS_EXPR and then perform a wi::add?
if (inner_op == MINUS_EXPR)
w1 = wi::neg (w1);
if (outer_op == MINUS_EXPR)
w2 = wi::neg (w2);
Yes.
I was concerned about the case where w1 or w2 might be the minimum
(negative) integer for its type. That creates a constant that can't be
represented. I'm not familiar enough with the rest of hte code yet to
know if that's problematical.
tree.c doesn't look like the best fit. I think putting it into
tree-vrp.c is better
and I think that extract_range_from_binary_expr_1 itself should
compute what we
want here as additional output. Conservative handling for all but
plus/minus is
ok with me.
I kept the extra function for now because I find
extract_range_from_binary_expr_1 somewhat lengthy and hard to follow
already :)
Heh, but I think duplicating code is even worse.
I was going to suggest a pre-patch that just refactored the various
cases in extract_range_from_binary_expr_1 into their own functions.
THat might it easier to keep things manageable.
[ ... ]
Now looking at binop_overflow (that I don't like very much, but ...)
Note that the naked "return true" in there makes 95% of that function
unreachable code. That's where I stopped without looking deeply at the
rest of the code.
Jeff
