[PATCH] D34028: [Bitcode] Add thumb-mode to target-features in metadata loader.

[Florian Hahn via llvm-commits]

On 19/06/2017 21:51, Adrian Prantl wrote:
> 
>> On Jun 19, 2017, at 1:08 PM, David Blaikie <dblaikie at gmail.com 
>> <mailto:dblaikie at gmail.com>> wrote:
>>
>>
>>
>> On Mon, Jun 19, 2017 at 8:57 AM Florian Hahn <florian.hahn at arm.com 
>> <mailto:florian.hahn at arm.com>> wrote:
>>
>>
>>
>>     On 09/06/2017 18:18, David Blaikie wrote:
>>     > I'd wonder if the code in ARMSubtarget could be refactored into
>>     > something generically reusable (for splitting and examining the
>>     > attributes) that could be used here and there, perhaps.
>>     >
>>
>>     I'll take a look and try to find similar uses.
>>
>>     >
>>     >     Re your second comment: yes the idea is that code generated
>>     by clang
>>     >     now sets +/-thumb-mode for all functions. For functions
>>     without a
>>     >     thumb-mode target feature and  a thumb triple, Thumb code
>>     should be
>>     >     generated, thus +thumb-mode is added.
>>     >
>>     >
>>     > Should this be a verifier check/invariant that modern bitcode (I
>>     guess
>>     > there's no version number, so no way to identify the old versus
>>     the new)
>>     > & textual IR have +/-thumb on all functions?
>>
>>     I think it would make sense to add a check to the verifier. Should
>>     I do
>>     that as part of this patch?
>>
>>
>> +Adrian - I know this is a bit out of his wheelhouse (he's been doing 
>> lots of improvements to the verifier, but mostly around debug info). 
>> But perhaps he's got some ideas about what might be the right approach 
>> here, or not. (or maybe can point the right person at it)
> 
> We don't really are checking any target-specific attributes in the 
> Verifier right now, but I think adding it there would make sense. There 
> is a lot of other code for checking various function attributes. 
> Alternatively this could also be an error condition in the 
> BitcodeReader, but the Verifier seems cleaner/nicer to me.
> What I'm wondering about is how target-specific verification should be 
> handled. Should the Verifier call out into a verify hook in the target? 
> Should we check for ARM attributes in a compiler that is X86-only?
> 

I agree that a verifier check would be nice, but it seems like we do not 
have any access to targets specific information in lib/Bitcode and we 
should probably keep it that way.

With respect to the original patch, would it be OK to update the 
verifier in a separate patch or is this blocking this patch?

Cheers,
Florian