<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 19, 2017, at 1:08 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, Jun 19, 2017 at 8:57 AM Florian Hahn <<a href="mailto:florian.hahn@arm.com" class="">florian.hahn@arm.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><br class=""><br class="">On 09/06/2017 18:18, David Blaikie wrote:<br class="">> I'd wonder if the code in ARMSubtarget could be refactored into<br class="">> something generically reusable (for splitting and examining the<br class="">> attributes) that could be used here and there, perhaps.<br class="">><br class=""><br class="">I'll take a look and try to find similar uses.<br class=""><br class="">><br class="">> Re your second comment: yes the idea is that code generated by clang<br class="">> now sets +/-thumb-mode for all functions. For functions without a<br class="">> thumb-mode target feature and a thumb triple, Thumb code should be<br class="">> generated, thus +thumb-mode is added.<br class="">><br class="">><br class="">> Should this be a verifier check/invariant that modern bitcode (I guess<br class="">> there's no version number, so no way to identify the old versus the new)<br class="">> & textual IR have +/-thumb on all functions?<br class=""><br class="">I think it would make sense to add a check to the verifier. Should I do<br class="">that as part of this patch?<br class=""></blockquote><div class=""><br class="">+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)<br class=""></div></div></div></div></blockquote><div><br class=""></div><div>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.</div><div>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?</div><div><br class=""></div>-- adrian</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><br class="">><br class="">> I'm not sure how situations like this are usually handled in the<br class="">> verifier/upgrade where the semantics haven't changed, but it won't<br class="">> roundtrip through bitcode as such - it'll get sort of upgraded to a<br class="">> newer representation of the same semantics.<br class="">><br class="">> Maybe that's fine... I dunno.<br class="">><br class=""><br class="">Do you know anyone who could know?<br class="">><br class="">><br class="">> ================<br class="">> Comment at: test/Bitcode/thumb-mode-upgrade-arm.ll:1<br class="">> +; RUN: llvm-dis -o - %s.bc | FileCheck %s<br class="">> +<br class="">> ----------------<br class="">> javed.absar wrote:<br class="">> > Could the same check be achieved using something like 'llvm-as <<br class="">> %s | llvm-dis ..."?<br class="">> I think the reason to include a bitcode file is to make sure older<br class="">> versions of the bitcode format are handled correctly going forward<br class="">><br class="">><br class="">> <a href="https://reviews.llvm.org/D34028" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D34028</a><br class="">><br class="">><br class="">></blockquote></div></div></div></blockquote></div><br class=""></body></html>