On 9/1/23 02:40, Andrew Pinski wrote:
On Fri, Aug 11, 2023 at 8:08 AM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:

If this is only going to work with integers, you might want to check
that somewhere or switch to irange and int_range_max..

You can make it work with any kind (if you know op1 is a constant) by
simply doing

Value_Range op1_range (TREE_TYPE (op1))
get_global_range_query->range_of_expr (op1_range, op1)

That will convert trees to a the appropriate range...  THis is also true
for integer constants... but you can also just do the WI conversion like
you do.

The routine also get confusing to read because it passes in op0 and
op1,  but of course ranger uses op1 and op2 nomenclature, and it looks a
bit confusing :-P   I'd change the operands passed in to op1 and op2 if
we are rewriting the routine.
Ranger using the nomenclature of op1/op2 and gimple is inconsistent
with trees and other parts of GCC.
It seems like we have to live with this inconsistency now too.
Renaming things in this one function to op1/op2 might be ok but the
rest of the file uses op0/op1 too; most likely because it was
originally written before gimple.

I think it would be good to have this written in the coding style,
which way should we have it for new code; if we start at 0 or 1 for
operands. It might reduce differences based on who wrote which part
(and even to some extent when). I don't really care which one is
picked as long as we pick one.

Thanks,
Andrew Pinski

I certainly wont argue it would be good to be consistent, but of course its quite prevalent. Perhaps we should rewrite vr-values.cc to change the terminology in one patch?

long term some of it is likely to get absorbed into rangeops, and what isn't could/should be made vrange/irange  aware...  no one has gotten to it yet. we could change the terminology as the routines are reworked too...

Andrew


Reply via email to