[LLVMdev] Inconsistent third field in global_ctors (was Re: [llvm] r214321 - UseListOrder: Visit global values)
Duncan P. N. Exon Smith
dexonsmith at apple.com
Thu Jul 31 09:49:46 PDT 2014
> On 2014-Jul-30, at 18:05, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
>> On 2014-Jul-30, at 17:19, Reid Kleckner <rnk at google.com> wrote:
>>
>>> On Wed, Jul 30, 2014 at 3:46 PM, Rafael EspĂndola <rafael.espindola at gmail.com> wrote:
>>>> Looking a little deeper, it's not "old-style" exactly; rejecting this
>>>> isn't trivial.
>>>>
>>>> - According to LangRef, the third field is optional [1].
>>>>
>>>> [1]: http://llvm.org/docs/LangRef.html#the-llvm-global-ctors-global-variable
>>>>
>>>> - It was added in r209015 to support comdat keys.
>>>>
>>>> - The helper functions `llvm::appendToGlobalCtors()` and
>>>> `llvm::appendToGlobalDtors()` (in ModuleUtils.cpp) will by default
>>>> set up the two-field version.
>>>>
>>>> - Despite the field being optional (and the API defaulting to the
>>>> short version), the BitcodeReader auto-upgrades to the 3-element
>>>> version.
>>>>
>>>> It looks to me like `@llvm.global_ctors` and `@llvm.global_dtors` are in
>>>> an inconsistent state here. In particular, with the same version of the
>>>> tool, if you generate the arrays you get one type, then if you write to
>>>> bitcode and read it back you get another type.
>>>>
>>>> There are a few ways to move forward:
>>>>
>>>> 1. Remove the auto-upgrade from the BitcodeReader, making the third
>>>> field truly optional. Why create them one way, and round-trip to
>>>> bitcode another way?
>>>>
>>>> 2. Make the third field *required*. This primarily means:
>>>>
>>>> - changing `appendToGlobal?tors()` (etc.?) to add the third field
>>>> immediately instead of waiting for a bitcode rount-trip and
>>>> - rejecting the two-field version in .ll.
>>>>
>>>> This is another way of removing the inconsistency.
>>>>
>>>
>>> I have a small preference for 2, but just because it makes the current
>>> IR definition simpler. Reid implemented the auto upgrade, so he
>>> probably has a better insight on which option is best.
>>
>> Nick was in favor of 2 as well, which is what pushed me to do the auto-upgrade, even though I initially intended for this to be optional. I didn't want to force the complexity of the 3 operand form on non-C++ frontends, basically. Obviously, having one form is simpler from LLVM's perspective.
>
> I was selfishly hoping (1) was better (deleting code is easier), but I
> guess (2) is cleaner so if no one objects I'll move forward with that.
>
> I should be able to throw a patch together tomorrow.
Hold on, I'm still not sure what kind of guarantees (restrictions) the C
API imposes.
If I implement (2), then -verify will fail if someone (e.g.) adds
`@llvm.global_ctors` with 2 fields. Does C API stability preclude
making that change?
Assuming it does, are there serious concerns with me implementing (1)
instead?
More information about the llvm-dev
mailing list