https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119872
--- Comment #9 from rguenther at suse dot de <rguenther at suse dot de> --- On Mon, 21 Apr 2025, tnfchris at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119872 > > --- Comment #8 from Tamar Christina <tnfchris at gcc dot gnu.org> --- > (In reply to Richard Biener from comment #7) > > Please make sure to not "fix" something where the input is already wrong - > > see the various issues where SCEV produces an invalid CHREC - forming a > > chrec > > is association. > > > > Using twice wide types within the same context is wrong IMO, the intent of > > the previous change was to identify IVs that end up with the same sequence > > of IV values, and we should preserve that. I don't see whether this is > > actually a case where the IV values are not the same? It seems we're using > > one in place of the other in a context where the evolution expression itself > > is re-used (but the one with undefined overflow) and that's the wrong thing > > to do? > > That's fair I was operating under the impression the original code had a > reason > to do what it did. > > The more I look at it the more I'm not sure why it generated incorrect code as > at most I would have expected it to use another IV. So will look into that. > > That said the original constant_multiple_of is quite odd.. > > e.g. > > (rr) p debug (top) > 4294967292 as signed int this is -4 > $7 = void > (rr) p debug (bot) > 4294967295 and this -1 > > for non-poly it does > > case INTEGER_CST: > if (TREE_CODE (bot) != INTEGER_CST) > return false; > > p0 = widest_int::from (wi::to_wide (top), SIGNED); > p1 = widest_int::from (wi::to_wide (bot), SIGNED); > if (p1 == 0) > return false; > *mul = wi::sext (wi::divmod_trunc (p0, p1, SIGNED, &res), precision); > > > so it would return that those two are a multiple of one another and that mul > == > 4 which clearly is bogus if it's supposed to check multiples. which makes this look correct. Also (gdb) p (unsigned int)(4294967295 * 4) $3 = 4294967292 so even for unsigned it's correct. Note this relies on 'SIGNED' intepretation, otherwise the division/modulo results are "off". > for Poly it does what's expected: > > if (POLY_INT_CST_P (top) > && POLY_INT_CST_P (bot) > && constant_multiple_p (wi::to_poly_widest (top), > wi::to_poly_widest (bot), mul)) > return true; > > So this code is odd.. The poly implementation looks quite similar to me, how does it arrive at a different result? Because wi::to_poly_widest uses TYPE_SIGN and the modulo is thus computed non-zero?
