<div dir="ltr">Hi Steven,<div><br></div><div>Sorry about that. I'm out on vacation for another week, do you want to send a patch to fix in the meantime?</div><div><br></div><div>Teresa</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 7, 2017 at 10:16 AM, Steven Wu <span dir="ltr"><<a href="mailto:stevenwu@apple.com" target="_blank">stevenwu@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Teresa<br>
<br>
I think this commit introduces bitcode compatibility issue. Here is the situation:<br>
If I need to link a bitcode static library into my project. The library is built with old compiler which has PIC module flag:<br>
<span class="">!0 = !{i32 1, !"PIC Level", i32 2}<br>
</span>The LTO object I built with latest compiler will have:<br>
<span class="">!0 = !{i32 7, !"PIC Level", i32 2}<br>
</span>When linking, libLTO will error: linking module flags 'PIC Level': IDs have conflicting behaviors<br>
<br>
My suggestion will be add AutoUpgrade to upgrade PIC Level from old bitcode to Module::Max. This should be cleaner than a hack in the IRMover to create an exception for this case.<br>
<br>
Also, LangRef need to updated to add the new behavior.<br>
<span class="HOEnZb"><font color="#888888"><br>
Steven<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> On May 22, 2017, at 5:08 PM, Teresa Johnson via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Author: tejohnson<br>
> Date: Mon May 22 19:08:00 2017<br>
> New Revision: 303590<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=303590&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=303590&view=rev</a><br>
> Log:<br>
> Support for taking the max of module flags when linking, use for PIE/PIC<br>
><br>
> Summary:<br>
> Add Max ModFlagBehavior, which can be used to take the max of two<br>
> module flag values when merging modules. Use it for the PIE and PIC<br>
> levels.<br>
><br>
> This avoids an error when we try to import from a module built -fpic<br>
> into a module built -fPIC, for example. For both PIE and PIC levels,<br>
> this will be legal, since the code generation gets more conservative<br>
> as the level is increased. Therefore we can take the max instead of<br>
> somehow trying to block importing between modules compiled with<br>
> different levels.<br>
><br>
> Reviewers: tmsriram, pcc<br>
><br>
> Subscribers: llvm-commits<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D33418" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D33418</a><br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/IR/<wbr>Module.h<br>
> llvm/trunk/lib/IR/Module.cpp<br>
> llvm/trunk/lib/IR/Verifier.cpp<br>
> llvm/trunk/lib/Linker/IRMover.<wbr>cpp<br>
> llvm/trunk/test/Linker/Inputs/<wbr>module-flags-pic-2-b.ll<br>
> llvm/trunk/test/Linker/module-<wbr>flags-pic-2-a.ll<br>
> llvm/trunk/test/Verifier/<wbr>module-flags-1.ll<br>
><br>
> Modified: llvm/trunk/include/llvm/IR/<wbr>Module.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Module.h?rev=303590&r1=303589&r2=303590&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/IR/Module.h?rev=303590&<wbr>r1=303589&r2=303590&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/include/llvm/IR/<wbr>Module.h (original)<br>
> +++ llvm/trunk/include/llvm/IR/<wbr>Module.h Mon May 22 19:08:00 2017<br>
> @@ -139,9 +139,12 @@ public:<br>
> /// during the append operation.<br>
> AppendUnique = 6,<br>
><br>
> + /// Takes the max of the two values, which are required to be integers.<br>
> + Max = 7,<br>
> +<br>
> // Markers:<br>
> ModFlagBehaviorFirstVal = Error,<br>
> - ModFlagBehaviorLastVal = AppendUnique<br>
> + ModFlagBehaviorLastVal = Max<br>
> };<br>
><br>
> /// Checks if Metadata represents a valid ModFlagBehavior, and stores the<br>
><br>
> Modified: llvm/trunk/lib/IR/Module.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Module.cpp?rev=303590&r1=303589&r2=303590&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/IR/<wbr>Module.cpp?rev=303590&r1=<wbr>303589&r2=303590&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/IR/Module.cpp (original)<br>
> +++ llvm/trunk/lib/IR/Module.cpp Mon May 22 19:08:00 2017<br>
> @@ -481,7 +481,7 @@ PICLevel::Level Module::getPICLevel() co<br>
> }<br>
><br>
> void Module::setPICLevel(PICLevel::<wbr>Level PL) {<br>
> - addModuleFlag(ModFlagBehavior:<wbr>:Error, "PIC Level", PL);<br>
> + addModuleFlag(ModFlagBehavior:<wbr>:Max, "PIC Level", PL);<br>
> }<br>
><br>
> PIELevel::Level Module::getPIELevel() const {<br>
> @@ -495,7 +495,7 @@ PIELevel::Level Module::getPIELevel() co<br>
> }<br>
><br>
> void Module::setPIELevel(PIELevel::<wbr>Level PL) {<br>
> - addModuleFlag(ModFlagBehavior:<wbr>:Error, "PIE Level", PL);<br>
> + addModuleFlag(ModFlagBehavior:<wbr>:Max, "PIE Level", PL);<br>
> }<br>
><br>
> void Module::setProfileSummary(<wbr>Metadata *M) {<br>
><br>
> Modified: llvm/trunk/lib/IR/Verifier.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=303590&r1=303589&r2=303590&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/IR/<wbr>Verifier.cpp?rev=303590&r1=<wbr>303589&r2=303590&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/IR/Verifier.cpp (original)<br>
> +++ llvm/trunk/lib/IR/Verifier.cpp Mon May 22 19:08:00 2017<br>
> @@ -1282,6 +1282,13 @@ Verifier::visitModuleFlag(<wbr>const MDNode *<br>
> // These behavior types accept any value.<br>
> break;<br>
><br>
> + case Module::Max: {<br>
> + Assert(mdconst::dyn_extract_<wbr>or_null<ConstantInt>(Op-><wbr>getOperand(2)),<br>
> + "invalid value for 'max' module flag (expected constant integer)",<br>
> + Op->getOperand(2));<br>
> + break;<br>
> + }<br>
> +<br>
> case Module::Require: {<br>
> // The value should itself be an MDNode with two operands, a flag ID (an<br>
> // MDString), and a value.<br>
><br>
> Modified: llvm/trunk/lib/Linker/IRMover.<wbr>cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=303590&r1=303589&r2=303590&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/Linker/<wbr>IRMover.cpp?rev=303590&r1=<wbr>303589&r2=303590&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/Linker/IRMover.<wbr>cpp (original)<br>
> +++ llvm/trunk/lib/Linker/IRMover.<wbr>cpp Mon May 22 19:08:00 2017<br>
> @@ -1157,6 +1157,11 @@ Error IRLinker::<wbr>linkModuleFlagsMetadata(<br>
> mdconst::extract<ConstantInt>(<wbr>DstOp->getOperand(0));<br>
> unsigned DstBehaviorValue = DstBehavior->getZExtValue();<br>
><br>
> + auto overrideDstValue = [&]() {<br>
> + DstModFlags->setOperand(<wbr>DstIndex, SrcOp);<br>
> + Flags[ID].first = SrcOp;<br>
> + };<br>
> +<br>
> // If either flag has override behavior, handle it first.<br>
> if (DstBehaviorValue == Module::Override) {<br>
> // Diagnose inconsistent flags which both have override behavior.<br>
> @@ -1167,8 +1172,7 @@ Error IRLinker::<wbr>linkModuleFlagsMetadata(<br>
> continue;<br>
> } else if (SrcBehaviorValue == Module::Override) {<br>
> // Update the destination flag to that of the source.<br>
> - DstModFlags->setOperand(<wbr>DstIndex, SrcOp);<br>
> - Flags[ID].first = SrcOp;<br>
> + overrideDstValue();<br>
> continue;<br>
> }<br>
><br>
> @@ -1204,6 +1208,15 @@ Error IRLinker::<wbr>linkModuleFlagsMetadata(<br>
> }<br>
> continue;<br>
> }<br>
> + case Module::Max: {<br>
> + ConstantInt *DstValue =<br>
> + mdconst::extract<ConstantInt>(<wbr>DstOp->getOperand(2));<br>
> + ConstantInt *SrcValue =<br>
> + mdconst::extract<ConstantInt>(<wbr>SrcOp->getOperand(2));<br>
> + if (SrcValue->getZExtValue() > DstValue->getZExtValue())<br>
> + overrideDstValue();<br>
> + break;<br>
> + }<br>
> case Module::Append: {<br>
> MDNode *DstValue = cast<MDNode>(DstOp-><wbr>getOperand(2));<br>
> MDNode *SrcValue = cast<MDNode>(SrcOp-><wbr>getOperand(2));<br>
><br>
> Modified: llvm/trunk/test/Linker/Inputs/<wbr>module-flags-pic-2-b.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/module-flags-pic-2-b.ll?rev=303590&r1=303589&r2=303590&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Linker/Inputs/module-flags-<wbr>pic-2-b.ll?rev=303590&r1=<wbr>303589&r2=303590&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/test/Linker/Inputs/<wbr>module-flags-pic-2-b.ll (original)<br>
> +++ llvm/trunk/test/Linker/Inputs/<wbr>module-flags-pic-2-b.ll Mon May 22 19:08:00 2017<br>
> @@ -1,3 +1,4 @@<br>
> -!0 = !{ i32 1, !"PIC Level", i32 2 }<br>
> +!0 = !{ i32 7, !"PIC Level", i32 2 }<br>
> +!1 = !{ i32 7, !"PIE Level", i32 2 }<br>
><br>
> -!llvm.module.flags = !{!0}<br>
> +!llvm.module.flags = !{!0, !1}<br>
><br>
> Modified: llvm/trunk/test/Linker/module-<wbr>flags-pic-2-a.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/module-flags-pic-2-a.ll?rev=303590&r1=303589&r2=303590&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Linker/module-flags-pic-2-a.<wbr>ll?rev=303590&r1=303589&r2=<wbr>303590&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/test/Linker/module-<wbr>flags-pic-2-a.ll (original)<br>
> +++ llvm/trunk/test/Linker/module-<wbr>flags-pic-2-a.ll Mon May 22 19:08:00 2017<br>
> @@ -1,10 +1,11 @@<br>
> -; RUN: not llvm-link %s %p/Inputs/module-flags-pic-2-<wbr>b.ll -S -o - 2> %t<br>
> -; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s<br>
> +; RUN: llvm-link %s %p/Inputs/module-flags-pic-2-<wbr>b.ll -S -o - | FileCheck %s<br>
><br>
> -; test linking modules with two different PIC levels<br>
> +; test linking modules with two different PIC and PIE levels<br>
><br>
> -!0 = !{ i32 1, !"PIC Level", i32 1 }<br>
> +!0 = !{ i32 7, !"PIC Level", i32 1 }<br>
> +!1 = !{ i32 7, !"PIE Level", i32 1 }<br>
><br>
> -!llvm.module.flags = !{!0}<br>
> +!llvm.module.flags = !{!0, !1}<br>
><br>
> -; CHECK-ERRORS: ERROR: linking module flags 'PIC Level': IDs have conflicting values<br>
> +; CHECK: !0 = !{i32 7, !"PIC Level", i32 2}<br>
> +; CHECK: !1 = !{i32 7, !"PIE Level", i32 2}<br>
><br>
> Modified: llvm/trunk/test/Verifier/<wbr>module-flags-1.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Verifier/module-flags-1.ll?rev=303590&r1=303589&r2=303590&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Verifier/module-flags-1.ll?<wbr>rev=303590&r1=303589&r2=<wbr>303590&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/test/Verifier/<wbr>module-flags-1.ll (original)<br>
> +++ llvm/trunk/test/Verifier/<wbr>module-flags-1.ll Mon May 22 19:08:00 2017<br>
> @@ -41,6 +41,10 @@<br>
> ; CHECK-NOT: invalid value for 'append'-type module flag (expected a metadata node)<br>
> !18 = !{i32 5, !"flag-4", !{i32 57}}<br>
><br>
> +; Check that any 'max' module flags are valid.<br>
> +; CHECK: invalid value for 'max' module flag (expected constant integer)<br>
> +!19 = !{i32 7, !"max", !"max"}<br>
> +<br>
> ; Check that any 'require' module flags are valid.<br>
> ; CHECK: invalid requirement on flag, flag is not present in module<br>
> !11 = !{i32 3, !"bar", !{!"no-such-flag", i32 52}}<br>
> @@ -54,4 +58,4 @@<br>
><br>
> !llvm.module.flags = !{<br>
> !0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15,<br>
> - !16, !17, !18 }<br>
> + !16, !17, !18, !19 }<br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> 408-460-2413</td></tr></tbody></table></span></div>
</div>