On 2/23/23 03:36, Tamar Christina wrote:
> Hi Andrew,
>
>>>> Oh yeah, and in case you haven't figured it out on your own, you'll
>>>> have to remove WIDEN_MULT_EXPR from the range-ops init table.   This
>>>> non-standard mechanism only gets checked if there is no standard
>>>> range-op table entry for the tree code :-P
>>>>
>>> Hmm it looks like it'll work, but it keeps segfaulting in:
>>>
>>> bool
>>> range_op_handler::fold_range (vrange &r, tree type,
>>> 			      const vrange &lh,
>>> 			      const vrange &rh,
>>> 			      relation_trio rel) const
>>> {
>>>     gcc_checking_assert (m_valid);
>>>     if (m_int)
>>>       return m_int->fold_range (as_a <irange> (r), type,
>>> 			   as_a <irange> (lh),
>>> 			   as_a <irange> (rh), rel);
>>>
>>> while trying to call fold_range.
>>>
>>> But m_int is set to the right instance. Probably something I'm
>>> missing, I'll double check it all.
>>>
>> Hmm.  whats your class operator_widen_mult* look like? what are you
>> inheriting from?   Send me your patch and I'll have a look if you want. this is
>> somewhat  new territory :-)
> I've attached the patch, and my testcase is:
>
> int decMultiplyOp_zacc, decMultiplyOp_iacc;
> int *decMultiplyOp_lp;
> void decMultiplyOp() {
>    decMultiplyOp_lp = &decMultiplyOp_zacc;
>    for (; decMultiplyOp_lp < &decMultiplyOp_zacc + decMultiplyOp_iacc;
>         decMultiplyOp_lp++)
>      *decMultiplyOp_lp = 0;
> }
>
> And compiling with aarch64-none-elf-gcc -O2 zero.c -S -o - -Werror=stringop-overflow
>
> Also to explain a bit on why we're only seeing this now:
>
> The original sequence for most of the pipeline is based on a cast and multiplication
>
>    # RANGE [irange] long unsigned int [0, 2147483647][18446744071562067968, +INF]
>    _14 = (long unsigned intD.11) decMultiplyOp_iacc.2_13;
>    # RANGE [irange] long unsigned int [0, 8589934588][18446744065119617024, 18446744073709551612] NONZERO 0xfffffffffffffffc
>    _15 = _14 * 4;
>
> But things like widening multiply are quite common, so some ISAs have it on scalars as well, not just vectors.
> So there's a pass widening_mul that runs late for these targets.  This replaces the above with
>
>    # RANGE [irange] long unsigned int [0, 8589934588][18446744065119617024, 18446744073709551612] NONZERO 0xfffffffffffffffc
>    _15 = decMultiplyOp_iacc.2_13 w* 4;
>
> And copies over the final range from the original expression.
>
> After that there are passes like the warning passes that try to requery ranged to see if any optimization  has changed them.
> Before my attempt to support *w this would just return VARYING and it would only use the old range.
>
> Now however, without taking care to sign extend when appropriate the MIN range changes from a negative value to a large
> positive one when we increase the precision.  So passes that re-query late get the wrong range.  That's why for instance in this case
> we get an incorrect warning generated.
>
> Thanks for the help!
>
> Tamar
>
>> I cant imagine it being a linkage thing between the 2 files since the operator is
>> defined in another file and the address taken in this one?
>> that should work, but strange that cant make the call...
>>
>> Andrew

It is some sort of linkage/vtable thing.  The fix.diff patch applied on 
top of what you have will fix the fold issue. This'll do for now until I 
formalize how this is going to work goign forward.

Inheriting from operator_mult is also going to be hazardous because it 
also has an op1_range and op2_range...  you should at least define those 
and return VARYING to avoid other issues.  Same thing applies to 
widen_plus I think, and it has relation processing and other things as 
well.  Your widen operands are not what those classes expect, so I think 
you probably just want a fresh range operator.

It also looks like the mult operation is sign/zero extending both upper 
bounds, and neither lower bound..   I think that should be the LH upper 
and lower bound?

I've attached a second patch  (newversion.patch) which incorporates my 
fix, the fix to the sign of only op1's bounds,  as well as a 
simplification of the classes to not inherit from operator_mult/plus..   
I think this still does what you want?  and it wont get you into 
unexpected trouble later :-)

let me know if this is still doing what you are expecting...

Andrew