On Dec 14, 2011, at 9:11 AM, David A. Greene wrote:> Dan Gohman <gohman at apple.com> writes: > >> Unfortunately, the final code is actually broken. If adding %a >> and %b as i32 values would overflow, adding them as i64 values >> would produce a value for which bit 31 is not equal to bit 32, so >> the subsequent ashr would produce a value which is not -1 or 0, the >> subsequent add would produce a value which is not 0 or 1, and the >> icmp ult would return 0, and the udiv would have undefined >> behavior. And it's unconditional. > > I'm not following. If the promotion to i64 produces a different value, > then the nsw smeantic was violated, leading to undefined behavior, as > you note. That that point all bets are off. Divide by zero certainly > is a perfectly valid expression of undefined behavior. If we had a > delayed check we would have to put it somewhere before the udiv. We > would probably need some kind of fixup path _a_la_ IA64's check > instruction.The original code was well-behaved for all inputs. The final code has undefined behavior for some inputs. Fixup paths don't really seem to fit here. We're never going to want fixup paths in the final output, for example. And there is no way to fix up undefined behavior. Dan
Dan Gohman <gohman at apple.com> writes:>> I'm not following. If the promotion to i64 produces a different value, >> then the nsw smeantic was violated, leading to undefined behavior, as >> you note. That that point all bets are off. Divide by zero certainly >> is a perfectly valid expression of undefined behavior. If we had a >> delayed check we would have to put it somewhere before the udiv. We >> would probably need some kind of fixup path _a_la_ IA64's check >> instruction. > > The original code was well-behaved for all inputs. > The final code has undefined behavior for some inputs. > > Fixup paths don't really seem to fit here. We're never going > to want fixup paths in the final output, for example. And > there is no way to fix up undefined behavior.My point is that hoisting the div above its control dependence is illegal unless you have some way of mitigating the poison path. Or we define new "poison" IR operations as Rafael suggested. I'm sort of intrigued by that idea. It's similar to a check operation but we could define whatever semantics we want (produce a safe value if poison, etc.). Again, it's undefined behavior so we get to define it. -Dave
On Dec 19, 2011, at 10:23 AM, David A. Greene wrote:> Dan Gohman <gohman at apple.com> writes: > >>> I'm not following. If the promotion to i64 produces a different value, >>> then the nsw smeantic was violated, leading to undefined behavior, as >>> you note. That that point all bets are off. Divide by zero certainly >>> is a perfectly valid expression of undefined behavior. If we had a >>> delayed check we would have to put it somewhere before the udiv. We >>> would probably need some kind of fixup path _a_la_ IA64's check >>> instruction. >> >> The original code was well-behaved for all inputs. >> The final code has undefined behavior for some inputs. >> >> Fixup paths don't really seem to fit here. We're never going >> to want fixup paths in the final output, for example. And >> there is no way to fix up undefined behavior. > > My point is that hoisting the div above its control dependence is > illegal unless you have some way of mitigating the poison path. Or we > define new "poison" IR operations as Rafael suggested. I'm sort of > intrigued by that idea. It's similar to a check operation but we could > define whatever semantics we want (produce a safe value if poison, > etc.). > > Again, it's undefined behavior so we get to define it.Could you spell out what this would look like? My sense is that check operations are a hardware-oriented approach, and that thinking about this as a hardware architecture problem means ignoring the range of transformations that compilers want to do. Dan