On Tue, Jan 24, 2023 at 10:57:12AM -0500, Andrew MacLeod wrote:
> That is the way VREL_OTHER is implemented in the table.
>
> So the problem is not on the true side of the IF condition as in your
> example, its on the false side. We see this commonly in code like this
>
>
> if (x <= y) // FALSE side registers x > y
> blah()
> else if (x >= y) // FALSE side registers x < y
> blah()
> else
> // Here we get VREL_UNDEFINED.
In that case, I think the problem isn't in the intersection, which works
correctly, but in wherever we (again, for HONOR_NANS (type) only) make the
assumptions about the else branches.
In the above case, the first blah has VREL_LE, that is correct,
but the else of it (when the condition isn't true but is false)
isn't VREL_GT, but VREL_UNGT (aka VREL_OTHER).
So, for the HONOR_NANS (type) cases we need to adjust relation_negate
callers.
On trees, you can e.g. look at fold-const.cc (invert_tree_comparison),
though that one for HONOR_NANS (type) && flag_trapping_math punts in most
cases to preserve exceptions. But you can see there the !honor_nans
cases where it acts like relation_negate, and then the honor_nans cases,
where VREL_*:
VARYING UNDEFINED LT LE GT GE EQ NE LTGT ORDERED UNORDERED UNLT UNLE UNGT UNGE
UNEQ
negate to
UNDEFINED VARYING UNGE UNGT UNLE UNLT NE EQ UNEQ UNORDERED ORDERED GE GT LE LT
LTGT
Or, in the world where VREL_OTHER is a wildcard for
LTGT ORDERED UNORDERED UNLT UNLE UNGT UNGE UNEQ
the less useful
VARYING UNDEFINED LT LE GT GE EQ NE OTHER
negates to
UNDEFINED VARYING OTHER OTHER OTHER OTHER NE EQ OTHER
But I'm afraid the above has VREL_OTHER for too many important cases,
unlike intersect where it is for none unless VREL_OTHER is involved, or just
a few ones for union.
So, I wonder if we just shouldn't do it properly immediately and instead of
VREL_OTHER introduce those
VREL_LTGT, /* Less than or greater than. */
VREL_ORDERED, /* Operands not NAN. */
VREL_UNORDERED, /* One or both operands NAN. */
VREL_UNLT, /* Unordered or less than. */
VREL_UNLE, /* Unordered or less than or equal. */
VREL_UNGT, /* Unordered or greater than. */
VREL_UNGE, /* Unordered or greater than or equal. */
VREL_UNEQ /* Unordered or equal. */
In that case, rather than using relation_{negate,union,intersect} etc.
either those functions should take an argument whether it is a HONOR_NANS
(type) floating point or not and use two separate tables under the hood,
or have two sets of routines and choose which one to use in the callers.
I think one routine with an extra argument would be cleaner though.
The above would grow the tables quite a bit (though, we could for the
non-FP cases limit ourselves to a subset), but because all the VREL_*
constants are enumerals with small integers values and the arrays
should be (but aren't for some reason?) static just make the arrays
contain unsigned char and do the casts to the enum in the
relation_{negate,intersect,union} etc. wrappers.
Also, I think the rr_*_table arrays should be const, we don't want to change
them, right? And a few other arrays too, like relation_to_code.
BTW, with the above extra 8 codes we could use the ORDERED_EXPR etc. tree
codes there.
Jakub
