On Thu, 20 Jul 2017, Richard Biener wrote: > >> So for saturating types isn't the issue when @1 and @2 have opposite > >> sign and the inner multiply would have saturated? > > > > No, I think the only special case is @1 == @2 == -1, otherwise either @2 is > > 0 or 1, or @1 * @2 is larger in magnitude than @1 (unless @1 == 0), and > > folding doesn't conceal an intermediate saturation. > > Ok, but you don't check for @1 == @2 == -1 because you rely on > the subtree being folded
Hm, I don't see why you say that, the patch ensures that @2 != -1 (I agree with Marc that implicitly relying on other folding to happen is not quite ok) (for the avoidance of doubt, "the only special case" was meant in the context of saturating types here) > fixed-point math has other issues (but I think at least > has symmetric negative/positive ranges). I don't think so, n1169.pdf calls out that fixed-point _Fract types may have asymmetric range such that -1.0 is exactly representable but 1.0 is not. > >> > I think overflow in the constant multiplication is ok unless > >> > TYPE_OVERFLOW_SANITIZED > >> > (and can we have a ubsan testcase for that that would otherwise fail?). > >> > It's not that we introduce new overflow here. > > > > This is to allow deducing that X must be zero. Otherwise, exploiting > > undefined overflow allows to fold X * CST1 * CST2 to just 0 if CST1 * CST2 > > overflows (this is what Marc originally suggested), but shouldn't we > > rather keep X in the IR to later deduce that X is zero? > > I don't think we should care here. After all we could immediately fold > X * CST1 * CST2 to zero if CST1 * CST2 overflows and overflow invokes > undefined behavior, right in this reassoc pattern. Note we cannot simply check if CST1 * CST2 overflows, because in case their exact product is -INT_MIN (i.e. INT_MAX+1), the original expression doesn't overflow for X==-1 (except when CST1==INT_MIN && CST2==-1). This was previously mentioned by Marc; sorry for neglecting to mention this special case in my previous email. So the motivation to not to do the transform on overflow is twofold: - unclear how to handle CST1*CST2 == -INT_MIN, if at all; - otherwise, folding X*CST1*CST2 to literal zero would drop X from the IR; but if X is an SSA_NAME, we could instead deduce that X is zero and replace all dominated uses of X by zero (I don't know how real-world-relevant this is) > So -- I'd prefer to just give up for TYPE_SATURATING (as you say currently > it can't happen anyway because the only saturating types we have are > fixed-point), > thus remove the zero/minus_one checks (not worry about false positive in > ubsan). OK, I think that can be done (I assume you mean 'false negative in ubsan'), but in light of the above clarification, what change do you want for overflow? Alexander
