Hi,
Andrew MacLeod <amacl...@redhat.com> writes: > On 9/12/23 21:42, Jiufu Guo wrote: >> Hi, >> >> Richard Biener <rguent...@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. Thanks a lot for the explaination! "false" means no ranger returned: we cannot use the range argument after call. > > >> 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. "undefined" means the ranger is unusable. So, for this ranger, it seems only "undefined_p ()" can be checked, and it seems no other functions of this ranger can be called. I'm thinking that it may be ok to let "range_of_expr" return false if the "vr" is "undefined_p". I know this may change the meaning of "range_of_expr" slightly :) > > 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. Yeap, thanks for pointing out this! BR, Jeff (Jiufu Guo) > > Andrew > > Andrew