
<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Agree, but that wouldn’t solve the downgrade problem plus this is probably too late to fix LLVM 6.0 for all the isFast uses.<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Feb 9, 2018, at 10:05 AM, Matthias Braun <<a href="mailto:mbraun@apple.com" class="">mbraun@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html; charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">Just wanted to point out part of this even becoming a problem is the use of `isFast()`.</div><div class="">There should be warnings against using isFast() and the existing code should be changed to query specific flags instead...</div><div class=""><br class=""></div><div class="">- Matthias</div><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Feb 8, 2018, at 5:34 PM, Quentin Colombet via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html; charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi,<div class=""><br class=""></div><div class="">TL;DR</div><div class="">r317488 changed the way fast math flags are laid out in the bitcode and anyone compiling a pre-llvm-6.0 bitcode with llvm-6.0 will lose all the optimizations guarded by isFast and a pre-llvm-6.0 compiler compiling a llvm-6.0 bitcode will potentially generate incorrect code w.r.t. fast math expectations.</div><div class=""><br class=""></div><div class="">Should we bump the bitcode version because of that and have the autoupgrader properly rewrite the fast-math to preserve that semantic?</div><div class="">(I believe we should!)</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">* Context *</div><div class=""><br class=""></div><div class="">With <a href="https://reviews.llvm.org/D39304" class="">https://reviews.llvm.org/D39304</a> / r317488 we got rid of the umbrella UnsafeMath flag and introduced 3 more flags that better represent the different things that happen under fast-math.</div><div class=""><br class=""></div><div class="">From a bitcode perspective, this change looks like this:</div><div class="">Before r317488 we had 6 bits that respectively represented:</div><div class=""><br class=""></div><div class="">UnsafeMath</div><div class="">nnan</div><div class="">ninf</div><div class="">nsz</div><div class="">arcp</div><div class="">contract</div><div class="">*unset*</div><div class=""><br class=""></div><div class="">(The order may not match what is exactly in the bitcode.)</div><div class=""><br class=""></div><div class=""><div class="">After r317488 we had 7 bits that respectively represented:</div></div><div class=""><div class="">reassoc (-UnsafeMath- is gone)</div><div class="">nnan</div><div class="">ninf</div><div class="">nsz</div><div class="">arcp</div><div class="">contract</div></div><div class="">*afn* (new bit)</div><div class=""><br class=""></div><div class="">Before r317488, fast-math was true if UnsafeMath was true (this should also imply all the other flags are sets). After r317488, fast-math is true if all the bits are set, in particular the afn, new one, too.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">* Problem *</div><div class=""><br class=""></div><div class="">Given we currently have no way to check if a bitcode file has been generated pre-r317488 or post-r317488 that means that:</div><div class="">1. a post-r317488 compiler is going to skip any optimization guarded by isFast for all pre-r317488 bitcode file (remember the afn bit is not set here)</div><div class="">2. a pre-r317488 compiler is going to run any optimization guarded by unsafeAlgebra for any post-r317488 bitcode file that has the reassoc bit (remember we repurposed UnsafeMath)</div><div class=""><br class=""></div><div class="">Scenario #2 might be unlikely but we’re potentially breaking the semantic of the program. It is particularly dangerous because there is nothing that is going to tell us that we are in this situation “downgrade" situation.</div><div class="">#1 means that any code that uses unsafeMath is going to get a performance hit.</div><div class=""><br class=""></div><div class="">In other words, one scenario implies generating wrong code and the other, runtime performance regressions.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">* Feedback Needed *</div><div class=""><br class=""></div><div class="">I believe this change is big enough that it would be worth bumping the bitcode version so that the upgrader can do the right thing *before* we release it to the public with LLVM-6.0.</div><div class=""><br class=""></div><div class="">That being said, I don’t know what are the implications of such bump and if people really don’t care about the performance problem that might be okay. The silent downgrade path is however concerning.</div><div class=""><br class=""></div><div class="">Should we bump the bitcode version because of that change and have the autoupgrader properly rewrite the fast-math flags to </div><div class="">preserve the semantic and make sure there are no silent downgrade?</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">-Quentin</div></div>_______________________________________________<br class="">LLVM Developers mailing list<br class=""><a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br class=""></div></blockquote></div><br class=""></div></div></blockquote></div><br class=""></body></html>