[llvm] r303590 - Support for taking the max of module flags when linking, use for PIE/PIC

Steven Wu via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 7 10:16:05 PDT 2017


Hi Teresa

I think this commit introduces bitcode compatibility issue. Here is the situation:
If I need to link a bitcode static library into my project. The library is built with old compiler which has PIC module flag:
!0 = !{i32 1, !"PIC Level", i32 2}
The LTO object I built with latest compiler will have:
!0 = !{i32 7, !"PIC Level", i32 2}
When linking, libLTO will error: linking module flags 'PIC Level': IDs have conflicting behaviors

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.

Also, LangRef need to updated to add the new behavior.

Steven

> On May 22, 2017, at 5:08 PM, Teresa Johnson via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: tejohnson
> Date: Mon May 22 19:08:00 2017
> New Revision: 303590
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=303590&view=rev
> Log:
> Support for taking the max of module flags when linking, use for PIE/PIC
> 
> Summary:
> Add Max ModFlagBehavior, which can be used to take the max of two
> module flag values when merging modules. Use it for the PIE and PIC
> levels.
> 
> This avoids an error when we try to import from a module built -fpic
> into a module built -fPIC, for example. For both PIE and PIC levels,
> this will be legal, since the code generation gets more conservative
> as the level is increased. Therefore we can take the max instead of
> somehow trying to block importing between modules compiled with
> different levels.
> 
> Reviewers: tmsriram, pcc
> 
> Subscribers: llvm-commits
> 
> Differential Revision: https://reviews.llvm.org/D33418
> 
> Modified:
>    llvm/trunk/include/llvm/IR/Module.h
>    llvm/trunk/lib/IR/Module.cpp
>    llvm/trunk/lib/IR/Verifier.cpp
>    llvm/trunk/lib/Linker/IRMover.cpp
>    llvm/trunk/test/Linker/Inputs/module-flags-pic-2-b.ll
>    llvm/trunk/test/Linker/module-flags-pic-2-a.ll
>    llvm/trunk/test/Verifier/module-flags-1.ll
> 
> Modified: llvm/trunk/include/llvm/IR/Module.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Module.h?rev=303590&r1=303589&r2=303590&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/Module.h (original)
> +++ llvm/trunk/include/llvm/IR/Module.h Mon May 22 19:08:00 2017
> @@ -139,9 +139,12 @@ public:
>     /// during the append operation.
>     AppendUnique = 6,
> 
> +    /// Takes the max of the two values, which are required to be integers.
> +    Max = 7,
> +
>     // Markers:
>     ModFlagBehaviorFirstVal = Error,
> -    ModFlagBehaviorLastVal = AppendUnique
> +    ModFlagBehaviorLastVal = Max
>   };
> 
>   /// Checks if Metadata represents a valid ModFlagBehavior, and stores the
> 
> Modified: llvm/trunk/lib/IR/Module.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Module.cpp?rev=303590&r1=303589&r2=303590&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Module.cpp (original)
> +++ llvm/trunk/lib/IR/Module.cpp Mon May 22 19:08:00 2017
> @@ -481,7 +481,7 @@ PICLevel::Level Module::getPICLevel() co
> }
> 
> void Module::setPICLevel(PICLevel::Level PL) {
> -  addModuleFlag(ModFlagBehavior::Error, "PIC Level", PL);
> +  addModuleFlag(ModFlagBehavior::Max, "PIC Level", PL);
> }
> 
> PIELevel::Level Module::getPIELevel() const {
> @@ -495,7 +495,7 @@ PIELevel::Level Module::getPIELevel() co
> }
> 
> void Module::setPIELevel(PIELevel::Level PL) {
> -  addModuleFlag(ModFlagBehavior::Error, "PIE Level", PL);
> +  addModuleFlag(ModFlagBehavior::Max, "PIE Level", PL);
> }
> 
> void Module::setProfileSummary(Metadata *M) {
> 
> Modified: llvm/trunk/lib/IR/Verifier.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=303590&r1=303589&r2=303590&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Verifier.cpp (original)
> +++ llvm/trunk/lib/IR/Verifier.cpp Mon May 22 19:08:00 2017
> @@ -1282,6 +1282,13 @@ Verifier::visitModuleFlag(const MDNode *
>     // These behavior types accept any value.
>     break;
> 
> +  case Module::Max: {
> +    Assert(mdconst::dyn_extract_or_null<ConstantInt>(Op->getOperand(2)),
> +           "invalid value for 'max' module flag (expected constant integer)",
> +           Op->getOperand(2));
> +    break;
> +  }
> +
>   case Module::Require: {
>     // The value should itself be an MDNode with two operands, a flag ID (an
>     // MDString), and a value.
> 
> Modified: llvm/trunk/lib/Linker/IRMover.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=303590&r1=303589&r2=303590&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Linker/IRMover.cpp (original)
> +++ llvm/trunk/lib/Linker/IRMover.cpp Mon May 22 19:08:00 2017
> @@ -1157,6 +1157,11 @@ Error IRLinker::linkModuleFlagsMetadata(
>         mdconst::extract<ConstantInt>(DstOp->getOperand(0));
>     unsigned DstBehaviorValue = DstBehavior->getZExtValue();
> 
> +    auto overrideDstValue = [&]() {
> +      DstModFlags->setOperand(DstIndex, SrcOp);
> +      Flags[ID].first = SrcOp;
> +    };
> +
>     // If either flag has override behavior, handle it first.
>     if (DstBehaviorValue == Module::Override) {
>       // Diagnose inconsistent flags which both have override behavior.
> @@ -1167,8 +1172,7 @@ Error IRLinker::linkModuleFlagsMetadata(
>       continue;
>     } else if (SrcBehaviorValue == Module::Override) {
>       // Update the destination flag to that of the source.
> -      DstModFlags->setOperand(DstIndex, SrcOp);
> -      Flags[ID].first = SrcOp;
> +      overrideDstValue();
>       continue;
>     }
> 
> @@ -1204,6 +1208,15 @@ Error IRLinker::linkModuleFlagsMetadata(
>       }
>       continue;
>     }
> +    case Module::Max: {
> +      ConstantInt *DstValue =
> +          mdconst::extract<ConstantInt>(DstOp->getOperand(2));
> +      ConstantInt *SrcValue =
> +          mdconst::extract<ConstantInt>(SrcOp->getOperand(2));
> +      if (SrcValue->getZExtValue() > DstValue->getZExtValue())
> +        overrideDstValue();
> +      break;
> +    }
>     case Module::Append: {
>       MDNode *DstValue = cast<MDNode>(DstOp->getOperand(2));
>       MDNode *SrcValue = cast<MDNode>(SrcOp->getOperand(2));
> 
> Modified: llvm/trunk/test/Linker/Inputs/module-flags-pic-2-b.ll
> URL: 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
> ==============================================================================
> --- llvm/trunk/test/Linker/Inputs/module-flags-pic-2-b.ll (original)
> +++ llvm/trunk/test/Linker/Inputs/module-flags-pic-2-b.ll Mon May 22 19:08:00 2017
> @@ -1,3 +1,4 @@
> -!0 = !{ i32 1, !"PIC Level", i32 2 }
> +!0 = !{ i32 7, !"PIC Level", i32 2 }
> +!1 = !{ i32 7, !"PIE Level", i32 2 }
> 
> -!llvm.module.flags = !{!0}
> +!llvm.module.flags = !{!0, !1}
> 
> Modified: llvm/trunk/test/Linker/module-flags-pic-2-a.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/module-flags-pic-2-a.ll?rev=303590&r1=303589&r2=303590&view=diff
> ==============================================================================
> --- llvm/trunk/test/Linker/module-flags-pic-2-a.ll (original)
> +++ llvm/trunk/test/Linker/module-flags-pic-2-a.ll Mon May 22 19:08:00 2017
> @@ -1,10 +1,11 @@
> -; RUN: not llvm-link %s %p/Inputs/module-flags-pic-2-b.ll -S -o - 2> %t
> -; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
> +; RUN: llvm-link %s %p/Inputs/module-flags-pic-2-b.ll -S -o - | FileCheck %s
> 
> -; test linking modules with two different PIC levels
> +; test linking modules with two different PIC and PIE levels
> 
> -!0 = !{ i32 1, !"PIC Level", i32 1 }
> +!0 = !{ i32 7, !"PIC Level", i32 1 }
> +!1 = !{ i32 7, !"PIE Level", i32 1 }
> 
> -!llvm.module.flags = !{!0}
> +!llvm.module.flags = !{!0, !1}
> 
> -; CHECK-ERRORS: ERROR: linking module flags 'PIC Level': IDs have conflicting values
> +; CHECK: !0 = !{i32 7, !"PIC Level", i32 2}
> +; CHECK: !1 = !{i32 7, !"PIE Level", i32 2}
> 
> Modified: llvm/trunk/test/Verifier/module-flags-1.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Verifier/module-flags-1.ll?rev=303590&r1=303589&r2=303590&view=diff
> ==============================================================================
> --- llvm/trunk/test/Verifier/module-flags-1.ll (original)
> +++ llvm/trunk/test/Verifier/module-flags-1.ll Mon May 22 19:08:00 2017
> @@ -41,6 +41,10 @@
> ; CHECK-NOT: invalid value for 'append'-type module flag (expected a metadata node)
> !18 = !{i32 5, !"flag-4", !{i32 57}}
> 
> +; Check that any 'max' module flags are valid.
> +; CHECK: invalid value for 'max' module flag (expected constant integer)
> +!19 = !{i32 7, !"max", !"max"}
> +
> ; Check that any 'require' module flags are valid.
> ; CHECK: invalid requirement on flag, flag is not present in module
> !11 = !{i32 3, !"bar", !{!"no-such-flag", i32 52}}
> @@ -54,4 +58,4 @@
> 
> !llvm.module.flags = !{
>   !0, !1, !2, !3, !4, !5, !6, !7, !8, !9, !10, !11, !12, !13, !14, !15,
> -  !16, !17, !18 }
> +  !16, !17, !18, !19 }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list