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

Fri Feb 16 11:37:03 PST 2018

On Fri, Feb 16, 2018 at 8:26 AM, Mehdi AMINI <joker.eph at gmail.com> wrote:

>
>
> 2018-02-13 17:46 GMT-08:00 Mehdi AMINI <joker.eph at gmail.com>:
>
>>
>>
>> 2018-02-13 13:29 GMT-08:00 Quentin Colombet <qcolombet at apple.com>:
>>
>>> Hi Mehdi,
>>>
>>>
>>> On Feb 13, 2018, at 12:34 PM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>>
>>>
>>>
>>> 2018-02-08 17:34 GMT-08:00 Quentin Colombet via llvm-dev <
>>> llvm-dev at lists.llvm.org>:
>>>
>>>> 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.
>>>>
>>>
>>>
>>>
>>> Scenario #1 is unsupported AFAIK, unless I missed something the bitcode
>>> is not forward compatible: loading newer bitcode with an older LLVM has
>>> never been supported as far I can remember.
>>>
>>>
>>> I agree, but we cannot detect that this is the situation we are in and
>>> debugging would prove hard. That said, if we say we don’t care, so be it :).
>>>
>>
>> I just mean that in general many things can blow up when the bitcode
>> evolves, and we don't expect or support (as far as I know) llvm-3.9 to be
>> used to read/process bitcode from llvm-4.0 for instance.
>> (I remember implementing in the Apple LLVM a hard check and failure for
>> this: we don't even try)
>>
>>
>>
>>>
>>>
>>> Scenario #2 is very much like other performance regression when we drop
>>> old metadata (i.e. bitcode upgrade isn't performance proof in general but
>>> only "best effort", there have been multiple instance of this in the past).
>>>
>>>
>>> Usually IIRC we try not to version the bitcode at all this way (i.e.
>>> bitcode does not have a linear versioning that is regularly bumped) but
>>> instead make sure the encoding itself allows an easy upgrade. I.e. the
>>> encoding of the FMF should have been such that the reader can detect and
>>> upgrade to the new IR representation.
>>> Now this is too late here I guess, so bumping may be a possible
>>> trade-off.
>>>
>>>
>>> Do you think we should do it, or live with the performance drop?
>>>
>>
>> I'm not saying this shouldn't be done, I don't have a dog in this fight:
>> I was just providing another data point "as a matter of general policy" in
>> the past there have been other cases where we haven't been able (or rather
>> chose to not put effort) into preserving all the performance when upgrading
>> (I think TBAA comes to my mind right now, but other kind of metadata have
>> also been lost). Now every case is different, and the FMF can be considered
>> too important to be left on the table.
>> For example it would impact LTO when loading libraries built with Xcode9
>> and before whenever the next released post-r317488 come up.
>>
>>
>>
>>> I know this performance drop is not acceptable for our use cases, but I
>>> don’t want to impose our ruling on this.
>>>
>>>
>>> What about any bitcode shipped after r317488 but before the version
>>> bump? Not worth taking into account because of the short period of time?
>>>
>>>
>>> I was hoping we can ignore that period of time because it was not in any
>>> official LLVM release.
>>>
>>
> Need to at least make sure everyone knows what to expect, so the
> cherry-pick for the LLVM 6.0 release should include an entry in the Release
> Notes.
>
> Hans: this can only happen if you're still OK to cherry-pick in LLVM 6.0
> branch https://reviews.llvm.org/D43253
>
> Duncan, Steven, Peter, Teresa: this change can break LTO if we read back
> bitcode emitted from trunk since r317488 (last November), any concerns?
>

For our usage of ThinLTO, we don't reuse bitcode across different
revisions, so it wouldn't cause an issue. The LTO caching in
lib/LTO/LTO.cpp also encodes the llvm revision, so it shouldn't be an issue
there either.

Teresa

> Best,
>
> --
> Mehdi
>
>
>
>
>

-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180216/251ec089/attachment.html>