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

Fri Feb 16 08:26:00 PST 2018

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?

Best,

-- 
Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180216/1d83c8f5/attachment.html>