On 10/8/20 3:54 PM, Jakub Jelinek wrote:
On Thu, Oct 08, 2020 at 12:22:11PM +0200, Jakub Jelinek via Gcc-patches wrote:
Perhaps another way out of this would be document and enforce that
__builtin_c[lt]z{,l,ll} etc calls are undefined at zero, but C[TL]Z ifn
calls are defined there based on *_DEFINED_VALUE_AT_ZERO (*) == 2
Huh, that magic 2 is not obvious. I guess we should document the values
of this macro in the source somewhere:
defaults.h:
/* Indicate that CLZ and CTZ are undefined at zero. */
#ifndef CLZ_DEFINED_VALUE_AT_ZERO
#define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) 0
#endif
#ifndef CTZ_DEFINED_VALUE_AT_ZERO
#define CTZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) 0
#endif
The following patch implements that, i.e. __builtin_c?z* now take full
advantage of them being UB at zero, while the ifns are well defined at zero
if *_DEFINED_VALUE_AT_ZERO (*) == 2. That is what fixes PR94801.
Furthermore, to fix PR97312, if it is well defined at zero and the value at
zero is prec, we don't lower the maximum unless the argument is known to be
non-zero.
Heh. I was just fixing the gimple-range.cc version. Thanks.
BTW. There's no reason why the vr-values can't just call the
gimple_ranger::range_of_builtin_call. In the original implementation we
had vr-values call the ranger version and trap if they differed. I'm
pretty sure you can have vr-values call range_of_builtin_call with a
value_range, and things will get squashed down appropriately. We should
really only have one version of this. I'm not suggesting you do it, but
I wouldn't object to it ;-).
--- gcc/gimple-range.cc.jj 2020-10-08 11:55:25.498313173 +0200
+++ gcc/gimple-range.cc 2020-10-08 15:36:14.926945183 +0200
@@ -714,8 +730,14 @@ gimple_ranger::range_of_builtin_call (ir
// the maximum.
wide_int max = r.upper_bound ();
if (max == 0)
- break;
- maxi = wi::floor_log2 (max);
+ {
+ if (mini == -1)
+ maxi = -1;
+ else if (maxi == prec)
+ mini = prec;
+ }
+ else if (maxi != prec)
+ maxi = wi::floor_log2 (max);
Hmmm, if max == 0, that means the range is [0, 0], because if the
highest bound of r is 0, there's nothing left on the bottom but another
0 since R is unsigned. Is that what you meant?
I think there was a bug in translation here: It looks like the original
code did:
maxi = tree_floor_log2 (vr0->max ());
/* For vr0 [0, 0] give up. */
if (maxi == -1)
break;
so perhaps the above (prior to your change) should have been:
wide_int max = r.upper_bound ();
maxi = wi::floor_log2 (max);
// For 0 give up.
if (maxi == -1)
break;
You may want to adjust.
Again, thanks for working on this.
Aldy
