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

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 19 13:51:42 PDT 2017


> On Jun 19, 2017, at 1:08 PM, David Blaikie <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?

-- adrian

>  
> 
> >
> > I'm not sure how situations like this are usually handled in the
> > verifier/upgrade where the semantics haven't changed, but it won't
> > roundtrip through bitcode as such - it'll get sort of upgraded to a
> > newer representation of the same semantics.
> >
> > Maybe that's fine... I dunno.
> >
> 
> Do you know anyone who could know?
> >
> >
> >     ================
> >     Comment at: test/Bitcode/thumb-mode-upgrade-arm.ll:1
> >     +; RUN: llvm-dis -o - %s.bc | FileCheck %s
> >     +
> >     ----------------
> >     javed.absar wrote:
> >      > Could the same check be achieved  using something like 'llvm-as <
> >     %s | llvm-dis ..."?
> >     I think the reason to include a bitcode file is to make sure older
> >     versions of the bitcode format are handled correctly going forward
> >
> >
> >     https://reviews.llvm.org/D34028 <https://reviews.llvm.org/D34028>
> >
> >
> >

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170619/3e684c24/attachment.html>


More information about the llvm-commits mailing list