Re: [PATCH] Checking undefined_p before using the vr

[Andrew MacLeod <amacleod@redhat.com>]

On 9/12/23 21:42, Jiufu Guo wrote:
> Hi,
>
> Richard Biener <rguenther@suse.de> writes:
>
>> On Thu, 7 Sep 2023, Jiufu Guo wrote:
>>
>>> Hi,
>>>
>>> As discussed in PR111303:
>>>
>>> For pattern "(X + C) / N": "div (plus@3 @0 INTEGER_CST@1) INTEGER_CST@2)",
>>> Even if "X" has value-range and "X + C" does not overflow, "@3" may still
>>> be undefined. Like below example:
>>>
>>> _3 = _2 + -5;
>>> if (0 != 0)
>>>    goto <bb 3>; [34.00%]
>>> else
>>>    goto <bb 4>; [66.00%]
>>> ;;  succ:       3
>>> ;;              4
>>>
>>> ;; basic block 3, loop depth 0
>>> ;;  pred:       2
>>> _5 = _3 / 5;
>>> ;;  succ:       4
>>>
>>> The whole pattern "(_2 + -5 ) / 5" is in "bb 3", but "bb 3" would be
>>> unreachable (because "if (0 != 0)" is always false).
>>> And "get_range_query (cfun)->range_of_expr (vr3, @3)" is checked in
>>> "bb 3", "range_of_expr" gets an "undefined vr3". Where "@3" is "_5".
>>>
>>> So, before using "vr3", it would be safe to check "!vr3.undefined_p ()".
>>>
>>> Bootstrap & regtest pass on ppc64{,le} and x86_64.
>>> Is this ok for trunk?
>> OK, but I wonder why ->range_of_expr () doesn't return false for
>> undefined_p ()?  While "undefined" technically means we can treat
>> it as nonnegative_p (or not, maybe but maybe not both), we seem to
>> not want to do that.  So why expose it at all to ranger users
>> (yes, internally we in some places want to handle undefined).
> I guess, currently, it returns true and then lets the user check
> undefined_p, maybe because it tries to only return false if the
> type of EXPR is unsupported.

false is returned if no range can be calculated for any reason. The most 
common ones are unsupported types or in some cases, statements that are 
not understood.  FALSE means you cannot use the range being passed in.


> Let "range_of_expr" return false for undefined_p would save checking
> undefined_p again when using the APIs.
>
undefined is a perfectly acceptable range.  It can be used to represent 
either values which has not been initialized, or more frequently it 
identifies values that cannot occur due to conflicting/unreachable 
code.  VARYING means it can be any range, UNDEFINED means this is 
unusable, so treat it accordingly.  Its propagated like any other range.

The only reason you are having issues is you are then asking for the 
type of the range, and an undefined range currently has no type, for 
historical reasons.

Andrew

Andrew