[IR] make -warn-frame-size into a module attr
authorNick Desaulniers <ndesaulniers@google.com>
Thu, 10 Jun 2021 23:03:14 +0000 (16:03 -0700)
committerNick Desaulniers <ndesaulniers@google.com>
Thu, 10 Jun 2021 23:15:27 +0000 (16:15 -0700)
-Wframe-larger-than= is an interesting warning; we can't know the frame
size until PrologueEpilogueInsertion (PEI); very late in the compilation
pipeline.

-Wframe-larger-than= was propagated through CC1 as an -mllvm flag, then
was a cl::opt in LLVM's PEI pass; this meant it was dropped during LTO
and needed to be re-specified via -plugin-opt.

Instead, make it part of the IR proper as a module level attribute,
similar to D103048. Introduce -fwarn-stack-size CC1 option.

Reviewed By: rsmith, qcolombet

Differential Revision: https://reviews.llvm.org/D103928

13 files changed:
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/Wframe-larger-than.c [new file with mode: 0644]
clang/test/Frontend/backend-diagnostic.c
clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
llvm/include/llvm/IR/Module.h
llvm/lib/CodeGen/PrologEpilogInserter.cpp
llvm/lib/IR/Module.cpp
llvm/test/CodeGen/ARM/warn-stack.ll
llvm/test/CodeGen/X86/warn-stack.ll
llvm/test/Linker/warn-stack-frame.ll [new file with mode: 0644]

index 4b1ff8e..b469980 100644 (file)
@@ -293,6 +293,7 @@ VALUE_CODEGENOPT(StackAlignment    , 32, 0) ///< Overrides default stack
                                             ///< alignment, if not 0.
 VALUE_CODEGENOPT(StackProbeSize    , 32, 4096) ///< Overrides default stack
                                                ///< probe size, even if 0.
+VALUE_CODEGENOPT(WarnStackSize     , 32, UINT_MAX) ///< Set via -fwarn-stack-size.
 CODEGENOPT(NoStackArgProbe, 1, 0) ///< Set when -mno-stack-arg-probe is used
 CODEGENOPT(DebugStrictDwarf, 1, 1) ///< Whether or not to use strict DWARF info.
 CODEGENOPT(DebugColumnInfo, 1, 0) ///< Whether or not to use column information
index 1271a75..d13b9fe 100644 (file)
@@ -1488,6 +1488,9 @@ defm cxx_static_destructors : BoolFOption<"c++-static-destructors",
   PosFlag<SetTrue>>;
 def fsymbol_partition_EQ : Joined<["-"], "fsymbol-partition=">, Group<f_Group>,
   Flags<[CC1Option]>, MarshallingInfoString<CodeGenOpts<"SymbolPartition">>;
+def fwarn_stack_size_EQ : Joined<["-"], "fwarn-stack-size=">, Group<f_Group>,
+  Flags<[CC1Option]>,
+  MarshallingInfoInt<CodeGenOpts<"WarnStackSize">, "UINT_MAX">;
 
 defm memory_profile : OptInFFlag<"memory-profile", "Enable", "Disable", " heap memory profiling">;
 def fmemory_profile_EQ : Joined<["-"], "fmemory-profile=">,
index 7efc97c..c3a4b1b 100644 (file)
@@ -787,6 +787,8 @@ void CodeGenModule::Release() {
         getCodeGenOpts().StackProtectorGuardOffset);
   if (getCodeGenOpts().StackAlignment)
     getModule().setOverrideStackAlignment(getCodeGenOpts().StackAlignment);
+  if (getCodeGenOpts().WarnStackSize != UINT_MAX)
+    getModule().setWarnStackSize(getCodeGenOpts().WarnStackSize);
 
   getTargetCodeGenInfo().emitTargetMetadata(*this, MangledDeclNames);
 
index bcbb35f..ddec79a 100644 (file)
@@ -4828,8 +4828,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   if (Arg *A = Args.getLastArg(options::OPT_Wframe_larger_than_EQ)) {
     StringRef v = A->getValue();
-    CmdArgs.push_back("-mllvm");
-    CmdArgs.push_back(Args.MakeArgString("-warn-stack-size=" + v));
+    CmdArgs.push_back(Args.MakeArgString("-fwarn-stack-size=" + v));
     A->claim();
   }
 
diff --git a/clang/test/Driver/Wframe-larger-than.c b/clang/test/Driver/Wframe-larger-than.c
new file mode 100644 (file)
index 0000000..1786325
--- /dev/null
@@ -0,0 +1,15 @@
+// RUN: %clang -Wframe-larger-than=42 \
+// RUN:   -v -E - < /dev/null 2>&1 | FileCheck %s --check-prefix=ENABLE
+// RUN: %clang -Wframe-larger-than=42 -Wno-frame-larger-than= \
+// RUN:   -v -E - < /dev/null 2>&1 | FileCheck %s --check-prefix=DISABLE
+// RUN: %clang -Wframe-larger-than=42 -Wno-frame-larger-than= -Wframe-larger-than=43 \
+// RUN:   -v -E - < /dev/null 2>&1 | FileCheck %s --check-prefix=REENABLE
+//
+// TODO: we might want to look into being able to disable, then re-enable this
+// warning properly. We could have the driver turn -Wframe-larger-than=X into
+// -Wframe-larger-than -fwarn-stack-size=X.  Also, we should support
+// -Wno-frame-larger-than (no = suffix) like GCC.
+
+// ENABLE: cc1 {{.*}} -fwarn-stack-size=42
+// DISABLE: cc1 {{.*}} -fwarn-stack-size=42 {{.*}} -Wno-frame-larger-than=
+// REENABLE: cc1 {{.*}} -fwarn-stack-size=43 {{.*}} -Wno-frame-larger-than=
index 1fe4fdb..8ec3b90 100644 (file)
@@ -4,11 +4,11 @@
 // _PROMOTE_: Promote warning to error.
 // _IGNORE_: Drop backend warning.
 //
-// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin 2> %t.err
+// RUN: not %clang_cc1 %s -fwarn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin 2> %t.err
 // RUN: FileCheck < %t.err %s --check-prefix=REGULAR --check-prefix=ASM
-// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin -Werror=frame-larger-than= 2> %t.err
+// RUN: not %clang_cc1 %s -fwarn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin -Werror=frame-larger-than= 2> %t.err
 // RUN: FileCheck < %t.err %s --check-prefix=PROMOTE --check-prefix=ASM
-// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin -Wno-frame-larger-than= 2> %t.err
+// RUN: not %clang_cc1 %s -fwarn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin -Wno-frame-larger-than= 2> %t.err
 // RUN: FileCheck < %t.err %s --check-prefix=IGNORE --check-prefix=ASM
 //
 // RUN: %clang_cc1 %s -S -o - -triple=i386-apple-darwin -verify -no-integrated-as
index 332dd22..01b9ff5 100644 (file)
@@ -1,5 +1,5 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang_cc1 %s -mllvm -warn-stack-size=0 -emit-codegen-only -triple=i386-apple-darwin 2>&1 | FileCheck %s
+// RUN: %clang_cc1 %s -fwarn-stack-size=0 -emit-codegen-only -triple=i386-apple-darwin 2>&1 | FileCheck %s
 
 // TODO: Emit rich diagnostics for thunks and move this into the appropriate test file.
 // Until then, test that we fall back and display the LLVM backend diagnostic.
index 81e29d9..024abe0 100644 (file)
@@ -913,6 +913,10 @@ public:
   unsigned getOverrideStackAlignment() const;
   void setOverrideStackAlignment(unsigned Align);
 
+  /// Get/set the stack frame size threshold to warn on.
+  unsigned getWarnStackSize() const;
+  void setWarnStackSize(unsigned Threshold);
+
   /// @name Utility functions for querying and setting the build SDK version
   /// @{
 
index ca57248..e09face 100644 (file)
@@ -138,11 +138,6 @@ char PEI::ID = 0;
 
 char &llvm::PrologEpilogCodeInserterID = PEI::ID;
 
-static cl::opt<unsigned>
-WarnStackSize("warn-stack-size", cl::Hidden, cl::init((unsigned)-1),
-              cl::desc("Warn for stack size bigger than the given"
-                       " number"));
-
 INITIALIZE_PASS_BEGIN(PEI, DEBUG_TYPE, "Prologue/Epilogue Insertion", false,
                       false)
 INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
@@ -278,7 +273,9 @@ bool PEI::runOnMachineFunction(MachineFunction &MF) {
   // Warn on stack size when we exceeds the given limit.
   MachineFrameInfo &MFI = MF.getFrameInfo();
   uint64_t StackSize = MFI.getStackSize();
-  if (WarnStackSize.getNumOccurrences() > 0 && WarnStackSize < StackSize) {
+
+  unsigned Threshold = MF.getFunction().getParent()->getWarnStackSize();
+  if (StackSize > Threshold) {
     DiagnosticInfoStackSize DiagStackSize(F, StackSize);
     F.getContext().diagnose(DiagStackSize);
   }
index 9b95550..40d3401 100644 (file)
@@ -732,6 +732,17 @@ void Module::setOverrideStackAlignment(unsigned Align) {
   addModuleFlag(ModFlagBehavior::Error, "override-stack-alignment", Align);
 }
 
+unsigned Module::getWarnStackSize() const {
+  Metadata *MD = getModuleFlag("warn-stack-size");
+  if (auto *CI = mdconst::dyn_extract_or_null<ConstantInt>(MD))
+    return CI->getZExtValue();
+  return UINT_MAX;
+}
+
+void Module::setWarnStackSize(unsigned Threshold) {
+  addModuleFlag(ModFlagBehavior::Error, "warn-stack-size", Threshold);
+}
+
 void Module::setSDKVersion(const VersionTuple &V) {
   SmallVector<unsigned, 3> Entries;
   Entries.push_back(V.getMajor());
index 6756a4b..156bfbf 100644 (file)
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple thumbv7-apple-ios3.0.0 -warn-stack-size=80 < %s 2>&1 >/dev/null | FileCheck %s
+; RUN: llc -mtriple thumbv7-apple-ios3.0.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack size exceeds the
 ; given amount.
 ; <rdar://13987214>
@@ -22,3 +22,6 @@ entry:
 }
 
 declare void @doit(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
index 7353d07..cf1cd14 100644 (file)
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple x86_64-apple-macosx10.8.0 -warn-stack-size=80 < %s 2>&1 >/dev/null | FileCheck %s
+; RUN: llc -mtriple x86_64-apple-macosx10.8.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack size exceeds the
 ; given amount.
 ; <rdar://13987214>
@@ -22,3 +22,6 @@ entry:
 }
 
 declare void @doit(i8*)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
diff --git a/llvm/test/Linker/warn-stack-frame.ll b/llvm/test/Linker/warn-stack-frame.ll
new file mode 100644 (file)
index 0000000..504eb8f
--- /dev/null
@@ -0,0 +1,16 @@
+; RUN: split-file %s %t
+; RUN: llvm-link %t/main.ll %t/match.ll
+; RUN: not llvm-link %t/main.ll %t/mismatch.ll 2>&1 | \
+; RUN:   FileCheck --check-prefix=CHECK-MISMATCH %s
+
+; CHECK-MISMATCH: error: linking module flags 'warn-stack-size': IDs have conflicting values
+
+;--- main.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
+;--- match.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 80}
+;--- mismatch.ll
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"warn-stack-size", i32 81}