<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">OK I see now, your change enforce the “(exactly)” in the comment.<div class=""><br class=""></div><div class="">The commit LGTM.</div><div class=""><br class=""></div><div class="">Some comments:</div><div class=""><br class=""></div><div class="">I would still love to see a comment in the unit test file (as it is done earlier in the file for the other tests).</div><div class=""><br class=""></div><div class="">Also the unit test does not test the <span style="font-family: monospace, monospace;" class="">rmTowardNegative</span> case. It seems to be never tested by the validation. I changed</div><div class=""><span style="font-family: monospace, monospace;" class=""><br class=""></span></div><div class=""><span style="font-family: monospace, monospace;" class="">sign = (rounding_mode == rmTowardNegative);</span></div><div class=""><span style="font-family: monospace, monospace;" class=""><br class=""></span></div><div class="">for </div><div class=""><br class=""></div><div class=""><span style="font-family: monospace, monospace;" class="">sign = 0; </span></div><div class=""><span style="font-family: monospace, monospace;" class=""><br class=""></span></div><div class="">and ninja check-all is still passing.</div><div class=""><br class=""></div><div class="">Mehdi</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class="">On Jan 2, 2015, at 4:59 PM, Lang Hames <<a href="mailto:lhames@gmail.com" class="">lhames@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hi Mehdi,<div class=""><br class=""></div><div class="">Thanks for the feedback. I think the comment is fine - the problem is just that the original conditional didn't exactly match it. With the addition of the underflow check I believe the condition now matches the comment: The sign is only tweaked if the result of the addition is exactly zero, rather than a small result that truncates to zero.</div><div class=""><br class=""></div><div class="">CC'ing llvm-commits, where this email should have gone in the first place. :)<div class=""><br class=""></div></div><div class="">Cheers,</div><div class="">Lang.</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Jan 1, 2015 at 10:56 PM, Mehdi Amini <span dir="ltr" class=""><<a href="mailto:mehdi.amini@apple.com" target="_blank" class="">mehdi.amini@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="">Hi Lang,<div class=""><br class=""><div class=""><div class=""><div class="h5"><blockquote type="cite" class=""><div class="">On Jan 1, 2015, at 5:57 PM, Lang Hames <<a href="mailto:lhames@gmail.com" target="_blank" class="">lhames@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class="">Hi All,<div class=""><br class=""></div><div class="">APFloat::fusedMultiplyAdd currently computes the wrong signed zero when small negative results are truncated back to zero in standard precision. The following snippet handles the signedness in fusedMultiplyAdd:</div><div class=""><br class=""></div><font face="monospace, monospace" class="">/* If two numbers add (exactly) to zero, IEEE 754 decrees it is a<br class=""> positive zero unless rounding to minus infinity, except that <br class=""> adding two like-signed zeroes gives that zero. */</font><div class=""><font face="monospace, monospace" class="">if (category == fcZero && sign != addend.sign)<br class=""> sign = (rounding_mode == rmTowardNegative);</font><div class=""><br class=""></div><div class="">The test "category == fcZero" tells us that the result was zero after rounding back down to standard precision, but since the addition is carried out in extended precision this doesn't guarantee that the result of the addition was exactly zero (so the comment text may not apply). The attached patch adds a check for underflow during truncation which ensures the correct signedness of the result.</div></div></div></div></blockquote><div class=""><br class=""></div><div class=""><br class=""></div></div></div><div class="">According to what you describe ("the comment text may not apply"), I feel that the comment might be updated as well.</div><div class=""><br class=""></div><div class="">Best,</div><div class=""><br class=""></div><div class="">Mehdi</div></div><br class=""></div></div></blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></body></html>