[llvm-dev] [RFC] Should we bump the bitcode version in LLVM 6.0?

[Sanjay Patel via llvm-dev]

I agree about isFast(), but determining the minimal subset of flags needed
to replace those calls requires some thought.

I did start looking at replacing isFast() in the reassociation pass. In
theory, we should be able to do those transforms with just 'reassoc' now,
but I hit complications with intersecting FMF of different instructions.

Conservatively, we could just ask if everything but the new 'afn' is set
for folds that do not include mathlib or intrinsic math functions. That
could be an alternative to versioning the IR...unless the perf problems are
actually caused by not folding mathlib calls now?

As I said in D39304, I have no objection to versioning the IR, but I also
don't know if there's a cost/downside to that.


On Fri, Feb 9, 2018 at 11:05 AM, Matthias Braun via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Just wanted to point out part of this even becoming a problem is the use
> of `isFast()`.
> There should be warnings against using isFast() and the existing code
> should be changed to query specific flags instead...
>
> - Matthias
>
> On Feb 8, 2018, at 5:34 PM, Quentin Colombet via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Hi,
>
> TL;DR
> 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.
>
> Should we bump the bitcode version because of that and have the
> autoupgrader properly rewrite the fast-math to preserve that semantic?
> (I believe we should!)
>
>
> * Context *
>
> With https://reviews.llvm.org/D39304 / 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.
>
> From a bitcode perspective, this change looks like this:
> Before r317488 we had 6 bits that respectively represented:
>
> UnsafeMath
> nnan
> ninf
> nsz
> arcp
> contract
> *unset*
>
> (The order may not match what is exactly in the bitcode.)
>
> After r317488 we had 7 bits that respectively represented:
> reassoc (-UnsafeMath- is gone)
> nnan
> ninf
> nsz
> arcp
> contract
> *afn* (new bit)
>
> 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.
>
>
> * Problem *
>
> Given we currently have no way to check if a bitcode file has been
> generated pre-r317488 or post-r317488 that means that:
> 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)
> 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)
>
> 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.
> #1 means that any code that uses unsafeMath is going to get a performance
> hit.
>
> In other words, one scenario implies generating wrong code and the other,
> runtime performance regressions.
>
>
> * Feedback Needed *
>
> 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.
>
> 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.
>
> Should we bump the bitcode version because of that change and have the
> autoupgrader properly rewrite the fast-math flags to
> preserve the semantic and make sure there are no silent downgrade?
>
>
> Thanks,
> -Quentin
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180209/39a7b7f0/attachment.html>