On Mon, 13 Feb 2017, Jakub Jelinek wrote:
On Mon, Feb 13, 2017 at 09:59:12AM +0100, Richard Biener wrote:
On Sun, 12 Feb 2017, Marc Glisse wrote:
On Sun, 12 Feb 2017, Marc Glisse wrote:
On Tue, 7 Feb 2017, Jakub Jelinek wrote:
* tree-vrp.c (simplify_div_or_mod_using_ranges): If op1 is not
constant, but SSA_NAME with a known integer range, use the minimum
of that range instead of op1 to determine if modulo can be replaced
with its first operand.
Would it make sense to use something like the operand_less_p helper so we
would also handle an INTEGER_CST lhs?
Oops, operand_less_p is just a helper for compare_values_warnv and even that
one doesn't seem to use ranges, so there may not already be a nice function we
can call (?). The idea remains that reusing such code would help handle more
cases (it may even handle a few symbolic cases).
Yeah, we only have the compare_range_with_value / compare_ranges functions
and those require you to "expand" the value you have a range for first.
Nice, I was looking for them but I'd missed those functions.
I think we can do something like following, but not sure how you want to
actually simplify it using helpers. The only thing I can think of is
something like get_value_range that works on both SSA_NAMEs and
INTEGER_CSTs, but then it either has to allocate a new value range struct
for the INTEGER_CST case, or be more like extract_range_from_tree and
then it would copy the range for the SSA_NAME case, penalizing the code.
If allocating a new range for INTEGER_CST is too expensive (I wouldn't
expect it to matter, but I never profiled gcc), we could have it on the
stack:
value_range tmp;
value_range* vr0;
if(TREE_CODE(op0)==INTEGER_CST){
tmp.max=tmp.min=op0;
tmp.type=VR_RANGE;
vr0=&tmp;
} else {
vr0=get_value_range(op0);
}
// same for op1
if(TYPE_UNSIGNED(TREE_TYPE(op0))&&compare_ranges(LT_EXPR,vr0,vr1,&ovf))
// op0 % op1 is just op0
etc.
That's not so different from what you did, but compare_ranges doesn't
dismiss symbolic ranges (I am not sure there are any cases where that
would help here), and it feels like it is duplicating less code. On the
other hand I haven't tried to write the signed case, which may be uglier
with this version since we would have to generate a range for -op1, so
maybe your version is better?
(thanks for the patch)
--
Marc Glisse
