nickdesaulniers created this revision.
Herald added a reviewer: deadalnix.
Herald added subscribers: dexonsmith, hiraditya.
nickdesaulniers requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
Promote override-stack-alignment to use it.
The problem with this approach is:
it looks like it's not possible to create a module with such semantics
then test it properly with `llvm-link`. The reason is that `llvm-link`
is structured to start with an empty module ("Composite"), then link in
the first module specified on the command line, then link in the rest of
the modules specified. So we can't disambiguate between the initial
empty module being linked against the initial source file with the
MDNode vs 2 full modules where 1 is missing the MD node. (ie. for two
input source files, the main module linking logic is run twice, not
once).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D103851
Files:
clang/test/CodeGen/stackrealign-main.c
llvm/include/llvm-c/Core.h
llvm/include/llvm/IR/Module.h
llvm/lib/IR/Core.cpp
llvm/lib/IR/Module.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Linker/IRMover.cpp
llvm/test/Linker/stack-alignment.ll
Index: llvm/test/Linker/stack-alignment.ll
===================================================================
--- llvm/test/Linker/stack-alignment.ll
+++ llvm/test/Linker/stack-alignment.ll
@@ -1,22 +1,19 @@
; RUN: split-file %s %t
-; TODO: it seems that ModFlagBehavior::Error doesn't error when a module is
-; missing a module level attribute. IRLinker::linkModuleFlagsMetadata
-; seems to think this is ok. Debugging stack mismatch errors is not a fun
-; time.
-; RUN not llvm-link %t/main.ll %t/none.ll 2>&1 | FileCheck --check-prefix=NONE %s
+; RUN: not llvm-link %t/main.ll %t/none.ll 2>&1 | FileCheck --check-prefix=NONE %s
+; RUN: not llvm-link %t/none.ll %t/main.ll 2>&1 | FileCheck --check-prefix=NONE %s
; RUN: llvm-link %t/main.ll %t/8.ll
; RUN: not llvm-link %t/main.ll %t/16.ll 2>&1 | FileCheck --check-prefix=CHECK-16 %s
;--- main.ll
-; NONE: error: linking module flags 'override-stack-alignment': IDs have conflicting values
-; CHECK-16: error: linking module flags 'override-stack-alignment': IDs have conflicting values
+; NONE: error: linking module flags 'override-stack-alignment': IDs have conflicting behaviors
+; CHECK-16: error: linking module flags 'override-stack-alignment': IDs have conflicting behaviors
!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"override-stack-alignment", i32 8}
+!0 = !{i32 8, !"override-stack-alignment", i32 8}
;--- none.ll
;--- 8.ll
!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"override-stack-alignment", i32 8}
+!0 = !{i32 8, !"override-stack-alignment", i32 8}
;--- 16.ll
!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"override-stack-alignment", i32 16}
+!0 = !{i32 8, !"override-stack-alignment", i32 16}
Index: llvm/lib/Linker/IRMover.cpp
===================================================================
--- llvm/lib/Linker/IRMover.cpp
+++ llvm/lib/Linker/IRMover.cpp
@@ -1216,17 +1216,41 @@
/// Merge the linker flags in Src into the Dest module.
Error IRLinker::linkModuleFlagsMetadata() {
- // If the source module has no module flags, we are done.
- const NamedMDNode *SrcModFlags = SrcM->getModuleFlagsMetadata();
- if (!SrcModFlags)
+ NamedMDNode *SrcModFlags = SrcM->getModuleFlagsMetadata();
+ NamedMDNode *DstModFlags = DstM.getOrInsertModuleFlagsMetadata();
+
+ // If the source module has no module flags, we still need to scan the
+ // destination module for any ErrorWhenMissing, otherwise we are done.
+ if (!SrcModFlags) {
+ for (MDNode *Op : DstModFlags->operands()) {
+ ConstantInt *Behavior = mdconst::extract<ConstantInt>(Op->getOperand(0));
+ if (Behavior->getZExtValue() == Module::ErrorWhenMissing) {
+ MDString *ID = cast<MDString>(Op->getOperand(1));
+ return stringErr("linking module flags '" + ID->getString() +
+ "': IDs have conflicting behaviors in '" +
+ SrcM->getModuleIdentifier() + "' and '" +
+ DstM.getModuleIdentifier() + "'");
+ }
+ }
return Error::success();
+ }
// If the destination module doesn't have module flags yet, then just copy
- // over the source module's flags.
- NamedMDNode *DstModFlags = DstM.getOrInsertModuleFlagsMetadata();
+ // over the source module's flags. Make sure to check for any
+ // ErrorWhenMissing.
if (DstModFlags->getNumOperands() == 0) {
- for (unsigned I = 0, E = SrcModFlags->getNumOperands(); I != E; ++I)
- DstModFlags->addOperand(SrcModFlags->getOperand(I));
+ for (MDNode *Op : SrcModFlags->operands()) {
+ ConstantInt *Behavior = mdconst::extract<ConstantInt>(Op->getOperand(0));
+ if (Behavior->getZExtValue() == Module::ErrorWhenMissing) {
+ MDString *ID = cast<MDString>(Op->getOperand(1));
+ // TODO: main case is now failing.
+ return stringErr("linking module flags '" + ID->getString() +
+ "': IDs have conflicting behaviors in '" +
+ SrcM->getModuleIdentifier() + "' and '" +
+ DstM.getModuleIdentifier() + "'");
+ }
+ DstModFlags->addOperand(Op);
+ }
return Error::success();
}
@@ -1360,7 +1384,8 @@
case Module::Require:
case Module::Override:
llvm_unreachable("not possible");
- case Module::Error: {
+ case Module::Error:
+ case Module::ErrorWhenMissing: {
// Emit an error if the values differ.
if (SrcOp->getOperand(2) != DstOp->getOperand(2))
return stringErr("linking module flags '" + ID->getString() +
Index: llvm/lib/IR/Verifier.cpp
===================================================================
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1561,6 +1561,7 @@
case Module::Error:
case Module::Warning:
case Module::Override:
+ case Module::ErrorWhenMissing:
// These behavior types accept any value.
break;
Index: llvm/lib/IR/Module.cpp
===================================================================
--- llvm/lib/IR/Module.cpp
+++ llvm/lib/IR/Module.cpp
@@ -729,7 +729,8 @@
}
void Module::setOverrideStackAlignment(unsigned Align) {
- addModuleFlag(ModFlagBehavior::Error, "override-stack-alignment", Align);
+ addModuleFlag(ModFlagBehavior::ErrorWhenMissing, "override-stack-alignment",
+ Align);
}
void Module::setSDKVersion(const VersionTuple &V) {
Index: llvm/lib/IR/Core.cpp
===================================================================
--- llvm/lib/IR/Core.cpp
+++ llvm/lib/IR/Core.cpp
@@ -322,6 +322,8 @@
return Module::ModFlagBehavior::Append;
case LLVMModuleFlagBehaviorAppendUnique:
return Module::ModFlagBehavior::AppendUnique;
+ case LLVMModuleFlagBehaviorErrorWhenMissing:
+ return Module::ModFlagBehavior::ErrorWhenMissing;
}
llvm_unreachable("Unknown LLVMModuleFlagBehavior");
}
@@ -341,6 +343,8 @@
return LLVMModuleFlagBehaviorAppend;
case Module::ModFlagBehavior::AppendUnique:
return LLVMModuleFlagBehaviorAppendUnique;
+ case Module::ModFlagBehavior::ErrorWhenMissing:
+ return LLVMModuleFlagBehaviorErrorWhenMissing;
default:
llvm_unreachable("Unhandled Flag Behavior");
}
Index: llvm/include/llvm/IR/Module.h
===================================================================
--- llvm/include/llvm/IR/Module.h
+++ llvm/include/llvm/IR/Module.h
@@ -116,7 +116,8 @@
/// This enumeration defines the supported behaviors of module flags.
enum ModFlagBehavior {
/// Emits an error if two values disagree, otherwise the resulting value is
- /// that of the operands.
+ /// that of the operands. Does not produce an error if one module is
+ /// missing a value.
Error = 1,
/// Emits a warning if two values disagree. The result value will be the
@@ -148,9 +149,13 @@
/// Takes the max of the two values, which are required to be integers.
Max = 7,
+ /// Emits an error if two values disagree, or when a value is specified for
+ /// one module, but not another.
+ ErrorWhenMissing = 8,
+
// Markers:
ModFlagBehaviorFirstVal = Error,
- ModFlagBehaviorLastVal = Max
+ ModFlagBehaviorLastVal = ErrorWhenMissing
};
/// Checks if Metadata represents a valid ModFlagBehavior, and stores the
Index: llvm/include/llvm-c/Core.h
===================================================================
--- llvm/include/llvm-c/Core.h
+++ llvm/include/llvm-c/Core.h
@@ -398,7 +398,8 @@
typedef enum {
/**
* Emits an error if two values disagree, otherwise the resulting value is
- * that of the operands.
+ * that of the operands. Does not error if one module has a value but the
+ * other does not.
*
* @see Module::ModFlagBehavior::Error
*/
@@ -444,6 +445,14 @@
* @see Module::ModFlagBehavior::AppendUnique
*/
LLVMModuleFlagBehaviorAppendUnique,
+
+ /**
+ * Emits an error if two values disagree, or when a value is specified for
+ * one module, but not another.
+ *
+ * @see Module::ModFlagBehavior::ErrorWhenMissing
+ */
+ LLVMModuleFlagBehaviorErrorWhenMissing,
} LLVMModuleFlagBehavior;
/**
Index: clang/test/CodeGen/stackrealign-main.c
===================================================================
--- clang/test/CodeGen/stackrealign-main.c
+++ clang/test/CodeGen/stackrealign-main.c
@@ -18,5 +18,5 @@
// CHECK-NOT: "stackrealign"
// CHECK: }
// CHECK: attributes [[MAIN]] = { noinline nounwind optnone {{.*}}"stackrealign"{{.*}} }
-// CHECK: !{i32 1, !"override-stack-alignment", i32 64}
+// CHECK: !{i32 8, !"override-stack-alignment", i32 64}
// DEFAULT-NOT: "override-stack-alignment"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
