[CodeGen] Improve large stack frame diagnostic

[paulkirth]

Summary

Add statistics about how much memory is used, in variables, spills, and
unsafestack.

Issue #58168 describes some of the difficulty diagnosing stack size issues
identified by -Wframe-larger-than. D135488 addresses some of those issues by
giving developers a method to view the stack layout and thereby understand
where and how stack memory is used.

However, that solution requires an additional pass, when a short summary about
how the compiler has allocated stack memory can inform developers about where
they should investigate. When they need the complete context, D135488 can
provide them with a more comprehensive set of diagnostics.

[nickdesaulniers]

Cool! Do you have an example of the diagnostic? Hard to visualize from the modified CHECK lines.

[paulkirth]

w/o SafeStack:

warning: stack frame size (100) exceeds limit (80) in function 'warn'
20/100 (20%) spills, 80/100 (80.00%) variables

w/ SafeStack

warning: stack frame size (100) exceeds limit (80) in function 'warn_safestack'
20/100 (20%) spills, 80/100 (80.00%) variables, 60/100 (60.00%) UnsafeStack

The lines w/ the % are the new additions. Looking, though it should probably just read unsafe stack instead of UnsafeStack

[paulkirth]

Revise diagnostic message

[nickdesaulniers]

Looks helpful! Just some pedantic non-blocker comments about style.

llvm/lib/CodeGen/PrologEpilogInserter.cpp

287

uint64_t

https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

287–290

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

294

Please don't use auto.
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
int64_t to match the addition of MFI::getObjectSize.

295–300

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

302

Don't need llvm:: namespace prefix.

I think it's simpler to just use dbgs() everywhere?

304

1.0f

305

Sink computing this into the condition below that is it's only consumer?

306

https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

[paulkirth]

Those are good suggestions, so thanks for pointing those out. I'll upload a new version with those fixes when I get a chance.

[paulkirth]

Address comments

• use better casting discipline
• replace auto w/ concrete types
• update use of llvm::dbgs()
• change "UnsafeStack" to "unsafe stack" in tests
• update bracing to match LLVM sytle guide

[MaskRay]

llvm/lib/CodeGen/PrologEpilogInserter.cpp

305

Note: we should not emit messages to dbg() in non-assertion builds.
This was fixed by 945f6e65be0d20b3446e7c1537c64151de618af4

llvm/test/CodeGen/ARM/warn-stack.ll

27

We should not test dbgs messages. This was fixed by b97527f14459ef89bc94d652fcf94ba91b61b34c.

llvm/lib/CodeGen/PrologEpilogInserter.cpp

 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <functional>
 #include <limits>
if (MF.getFunction().hasFnAttribute("warn-stack-size")) {
     bool Failed = MF.getFunction()
                       .getFnAttribute("warn-stack-size")
                       .getValueAsString()
                       .getAsInteger(10, Threshold);
     // Verifier should have caught this.
     assert(!Failed && "Invalid warn-stack-size fn attr value");
     (void)Failed;
   }
-  if (MF.getFunction().hasFnAttribute(Attribute::SafeStack)) {
-    StackSize += MFI.getUnsafeStackSize();
-  }
+  uint64_t UnsafeStackSize = MFI.getUnsafeStackSize();
+  if (MF.getFunction().hasFnAttribute(Attribute::SafeStack))
+    StackSize += UnsafeStackSize;
+
   if (StackSize > Threshold) {
     DiagnosticInfoStackSize DiagStackSize(F, StackSize, Threshold, DS_Warning);
     F.getContext().diagnose(DiagStackSize);
+    int64_t SpillSize = 0;
+    for (int Idx = MFI.getObjectIndexBegin(), End = MFI.getObjectIndexEnd();
+         Idx != End; ++Idx) {
+      if (MFI.isSpillSlotObjectIndex(Idx))
+        SpillSize += MFI.getObjectSize(Idx);
+    }
+
+    float SpillPct =
+        static_cast<float>(SpillSize) / static_cast<float>(StackSize);
+    float VarPct = 1.0f - SpillPct;
+    int64_t VariableSize = StackSize - SpillSize;
+    dbgs() << formatv("{0}/{1} ({3:P}) spills, {2}/{1} ({4:P}) variables",
+                      SpillSize, StackSize, VariableSize, SpillPct, VarPct);
+    if (UnsafeStackSize != 0) {
+      float UnsafePct =
+          static_cast<float>(UnsafeStackSize) / static_cast<float>(StackSize);
+      dbgs() << formatv(", {0}/{2} ({1:P}) unsafe stack", UnsafeStackSize,
+                        UnsafePct, StackSize);
     }
+    dbgs() << "\n";
+  }
+
   ORE->emit([&]() {
     return MachineOptimizationRemarkAnalysis(DEBUG_TYPE, "StackSize",
                                              MF.getFunction().getSubprogram(),
                                              &MF.front())
            << ore::NV("NumStackBytes", StackSize) << " stack bytes in function";
   });
 
   delete RS;

llvm/test/CodeGen/ARM/warn-stack.ll

 ; RUN: llc -mtriple thumbv7-apple-ios3.0.0 < %s 2>&1 >/dev/null | FileCheck %s
 ; Check the internal option that warns when the stack frame size exceeds the
 ; given amount.
 ; <rdar://13987214>
 
 ; CHECK-NOT: nowarn
 define void @nowarn() nounwind ssp "frame-pointer"="all" "warn-stack-size"="80" {
 entry:
   %buffer = alloca [12 x i8], align 1
-  %arraydecay = getelementptr inbounds [12 x i8], [12 x i8]* %buffer, i64 0, i64 0
-  call void @doit(i8* %arraydecay) nounwind
+  call void @doit(ptr %buffer) nounwind
   ret void
 }
 
-; CHECK: warning: stack frame size (92) exceeds limit (80) in function 'warn'
-define void @warn() nounwind ssp "frame-pointer"="all" "warn-stack-size"="80" {
+; CHECK: warning: stack frame size ([[STCK:[0-9]+]]) exceeds limit (80) in function 'warn'
+; CHECK: {{[0-9]+}}/[[STCK]] ({{.*}}%) spills, {{[0-9]+}}/[[STCK]] ({{.*}}%) variables
+define i32 @warn() nounwind ssp "frame-pointer"="all" "warn-stack-size"="80" {
 entry:
+  %var = alloca i32, align 4
   %buffer = alloca [80 x i8], align 1
-  %arraydecay = getelementptr inbounds [80 x i8], [80 x i8]* %buffer, i64 0, i64 0
-  call void @doit(i8* %arraydecay) nounwind
-  ret void
+  call void @doit(ptr %buffer) nounwind
+  call void @doit(ptr %var) nounwind
+  %val = load i32, ptr %var
+  ret i32 %val
+}
+
+; CHECK: warning: stack frame size ([[STCK:[0-9]+]]) exceeds limit (80) in function 'warn_safestack'
+; CHECK: {{[0-9]+}}/[[STCK]] ({{.*}}%) spills, {{[0-9]+}}/[[STCK]] ({{.*}}%) variables, {{[0-9]+}}/[[STCK]] ({{.*}}%) unsafe stack
+define i32 @warn_safestack() nounwind ssp safestack "warn-stack-size"="80" {
+entry:
+  %var = alloca i32, align 4
+  %a = alloca i32, align 4
+  %buffer = alloca [80 x i8], align 1
+  call void @doit(ptr %buffer) nounwind
+  call void @doit(ptr %var) nounwind
+  %val = load i32, ptr %var
+  ret i32 %val
 }
 
-declare void @doit(i8*)
+declare void @doit(ptr)

llvm/test/CodeGen/X86/warn-stack.ll

 ; 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 frame size exceeds the
 ; given amount.
 ; <rdar://13987214>
 
 ; CHECK-NOT: nowarn
 define void @nowarn() nounwind ssp "warn-stack-size"="80" {
 entry:
   %buffer = alloca [12 x i8], align 1
   call void @doit(ptr %buffer) nounwind
   ret void
 }
 
-; CHECK: warning: stack frame size (88) exceeds limit (80) in function 'warn'
+; CHECK: warning: stack frame size ([[STCK:[0-9]+]]) exceeds limit (80) in function 'warn'
+; CHECK: {{[0-9]+}}/[[STCK]] ({{.*}}%) spills, {{[0-9]+}}/[[STCK]] ({{.*}}%) variables
 define void @warn() nounwind ssp "warn-stack-size"="80" {
 entry:
   %buffer = alloca [80 x i8], align 1
   call void @doit(ptr %buffer) nounwind
   ret void
 }
 
 ; Ensure that warn-stack-size also considers the size of the unsafe stack.
 ; With safestack enabled the machine stack size is well below 80, but the
 ; combined stack size of the machine stack and unsafe stack will exceed the
 ; warning threshold
 
-; CHECK: warning: stack frame size (120) exceeds limit (80) in function 'warn_safestack'
-define void @warn_safestack() nounwind ssp safestack "warn-stack-size"="80" {
+; CHECK: warning: stack frame size ([[STCK:[0-9]+]]) exceeds limit (80) in function 'warn_safestack'
+; CHECK: {{[0-9]+}}/[[STCK]] ({{.*}}%) spills, {{[0-9]+}}/[[STCK]] ({{.*}}%) variables, {{[0-9]+}}/[[STCK]] ({{.*}}%) unsafe stack
+define i32 @warn_safestack() nounwind ssp safestack "warn-stack-size"="80" {
 entry:
+  %var = alloca i32, align 4
   %buffer = alloca [80 x i8], align 1
   call void @doit(ptr %buffer) nounwind
-  ret void
+  call void @doit(ptr %var) nounwind
+  %val = load i32, ptr %var
+  ret i32 %val
 }
 declare void @doit(ptr)