Improve the diagnostic of DiagnosticInfoResourceLimit (and warn-stack-size in particular)

[MaskRay]

Summary

Before: warning: stack size limit exceeded (888) in main (the limit (-Wframe-larger-than) is not mentioned)
After: warning: stack frame size (888) exceeds limit (100) in function 'main'

[nickdesaulniers]

GCC trunk: warning: the frame size of 80 bytes is larger than 0 bytes [-Wframe-larger-than=]
Clang trunk: warning: stack frame size of 88 bytes in function 'bar' [-Wframe-larger-than]

looks like the frontend doesn't use this particular text at all?

[MaskRay]

In D104667#2831488, @nickdesaulniers wrote:

GCC trunk: warning: the frame size of 80 bytes is larger than 0 bytes [-Wframe-larger-than=]
Clang trunk: warning: stack frame size of 88 bytes in function 'bar' [-Wframe-larger-than]

looks like the frontend doesn't use this particular text at all?

This diagnostic is emitted by clang (clang/lib/CodeGen/CodeGenAction.cpp).

-Wframe-larger-than= also records the module flag metadata "warn-stack-size" which can be useful for LTO.

% fclang -Wframe-larger-than=100 a.c  -o x          
a.c:1:5: warning: stack frame size of 888 bytes in function 'main' [-Wframe-larger-than]
int main(){
    ^
1 warning generated.
% fclang -Wframe-larger-than=100 -flto=thin -fuse-ld=lld a.c -o x
ld.lld: warning: stack size limit exceeded (888) in main

[nickdesaulniers]

In D104667#2831505, @MaskRay wrote:

In D104667#2831488, @nickdesaulniers wrote:

GCC trunk: warning: the frame size of 80 bytes is larger than 0 bytes [-Wframe-larger-than=]
Clang trunk: warning: stack frame size of 88 bytes in function 'bar' [-Wframe-larger-than]

looks like the frontend doesn't use this particular text at all?

This diagnostic is emitted by clang (clang/lib/CodeGen/CodeGenAction.cpp).

-Wframe-larger-than= also records the module flag metadata "warn-stack-size" which can be useful for LTO.

% fclang -Wframe-larger-than=100 a.c  -o x          
a.c:1:5: warning: stack frame size of 888 bytes in function 'main' [-Wframe-larger-than]
int main(){
    ^
1 warning generated.
% fclang -Wframe-larger-than=100 -flto=thin -fuse-ld=lld a.c -o x
ld.lld: warning: stack size limit exceeded (888) in main

Can we try to make the FE match what's printed during LTO? It's odd that they differ, IMO. Also, GCC prints the threshold value, as does our backend; but the frontend still does not.

[MaskRay]

In D104667#2831507, @nickdesaulniers wrote:

In D104667#2831505, @MaskRay wrote:

In D104667#2831488, @nickdesaulniers wrote:

GCC trunk: warning: the frame size of 80 bytes is larger than 0 bytes [-Wframe-larger-than=]
Clang trunk: warning: stack frame size of 88 bytes in function 'bar' [-Wframe-larger-than]

looks like the frontend doesn't use this particular text at all?

This diagnostic is emitted by clang (clang/lib/CodeGen/CodeGenAction.cpp).

-Wframe-larger-than= also records the module flag metadata "warn-stack-size" which can be useful for LTO.

% fclang -Wframe-larger-than=100 a.c  -o x          
a.c:1:5: warning: stack frame size of 888 bytes in function 'main' [-Wframe-larger-than]
int main(){
    ^
1 warning generated.
% fclang -Wframe-larger-than=100 -flto=thin -fuse-ld=lld a.c -o x
ld.lld: warning: stack size limit exceeded (888) in main

Can we try to make the FE match what's printed during LTO? It's odd that they differ, IMO. Also, GCC prints the threshold value, as does our backend; but the frontend still does not.

How about warning: stack frame size (88) exceeds limit (80) in function 'warn' for "warn-stack-size" (currently a module flag metadata, will soon become a function attribute)?

I can make the clang diagnostic match this style, too.

[nickdesaulniers]

How about warning: stack frame size (88) exceeds limit (80) in function 'warn'

Sure. Would you mind adding the before/after text to the commit description as well, please?

[MaskRay]

stack size -> stack frame size

The former is ambiguous (it could mean the full stack size).

[nickdesaulniers]

clang/test/Misc/backend-resource-limit-diagnostics.cl

4

The value of the limit is still not printed though.

Oh, we don't print (0) if the limit is 0? Perhaps add a --check-prefix and additional RUN line that exercises a non-zero limit from the frontend?

[nickdesaulniers]

This will need to be rebased on https://reviews.llvm.org/rG8ace12130526f450c822ca232d1f865b247d7434 and the tests updated, potentially.

[MaskRay]

clang/test/Misc/backend-resource-limit-diagnostics.cl

4

0 means the diagnostic does not convey the limit information. It needs more refactoring to make this happen... and probably should not be the job of this patch...

[MaskRay]

rebase

clang/test/Misc/backend-resource-limit-diagnostics.cl

 // REQUIRES: amdgpu-registered-target
 // RUN: not %clang_cc1 -emit-codegen-only -triple=amdgcn-- %s 2>&1 | FileCheck %s
 
-// CHECK: error: local memory limit exceeded (480000) in use_huge_lds
+// CHECK: error: local memory (480000) exceeds limit in function 'use_huge_lds'
 kernel void use_huge_lds()
 {
     volatile local int huge[120000];
     huge[0] = 2;
 }

clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp

 // REQUIRES: x86-registered-target
 // 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.
 namespace frameSizeThunkWarning {
   struct A {
     virtual void f();
   };
 
   struct B : virtual A {
     virtual void f();
   };
 
   // CHECK: warning: stack frame size of {{[0-9]+}} bytes in function 'frameSizeThunkWarning::B::f'
-  // CHECK: warning: stack size limit exceeded ({{[0-9]+}}) in {{[^ ]+}}
+  // CHECK: warning: stack frame size ([[#]]) exceeds limit in function '_ZTv0_n12_N21frameSizeThunkWarning1B1fEv'
   void B::f() {
     volatile int x = 0; // Ensure there is stack usage.
   }
 }

llvm/include/llvm/IR/DiagnosticInfo.h

 };
 
 class DiagnosticInfoStackSize : public DiagnosticInfoResourceLimit {
   void anchor() override;
 public:
   DiagnosticInfoStackSize(const Function &Fn, uint64_t StackSize,
                           DiagnosticSeverity Severity = DS_Warning,
                           uint64_t StackLimit = 0)
-      : DiagnosticInfoResourceLimit(Fn, "stack size", StackSize, Severity,
+      : DiagnosticInfoResourceLimit(Fn, "stack frame size", StackSize, Severity,
                                     DK_StackSize, StackLimit) {}
 
   uint64_t getStackSize() const { return getResourceSize(); }
   uint64_t getStackLimit() const { return getResourceLimit(); }
 
   static bool classof(const DiagnosticInfo *DI) {
     return DI->getKind() == DK_StackSize;
   }

llvm/lib/CodeGen/PrologEpilogInserter.cpp

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 (StackSize > Threshold) {
-    DiagnosticInfoStackSize DiagStackSize(F, StackSize);
+    DiagnosticInfoStackSize DiagStackSize(F, StackSize, DS_Warning, Threshold);
     F.getContext().diagnose(DiagStackSize);
   }
   ORE->emit([&]() {
     return MachineOptimizationRemarkAnalysis(DEBUG_TYPE, "StackSize",
                                              MF.getFunction().getSubprogram(),
                                              &MF.front())
            << ore::NV("NumStackBytes", StackSize) << " stack bytes in function";
   });

llvm/lib/IR/DiagnosticInfo.cpp

 
 void DiagnosticInfoInlineAsm::print(DiagnosticPrinter &DP) const {
   DP << getMsgStr();
   if (getLocCookie())
     DP << " at line " << getLocCookie();
 }
 
 void DiagnosticInfoResourceLimit::print(DiagnosticPrinter &DP) const {
-  DP << getResourceName() << " limit";
-
+  DP << getResourceName() << " (" << getResourceSize() << ") exceeds limit";
   if (getResourceLimit() != 0)
-    DP << " of " << getResourceLimit();
-
-  DP << " exceeded (" <<  getResourceSize() << ") in " << getFunction();
+    DP << " (" << getResourceLimit() << ')';
+  DP << " in function '" << getFunction() << '\'';
 }
 
 void DiagnosticInfoDebugMetadataVersion::print(DiagnosticPrinter &DP) const {
   DP << "ignoring debug info with an invalid version (" << getMetadataVersion()
      << ") in " << getModule();
 }
 
 void DiagnosticInfoIgnoringInvalidDebugMetadata::print(

llvm/test/CodeGen/AMDGPU/exceed-max-sgprs.ll

 ; RUN: not llc -march=amdgcn -verify-machineinstrs < %s 2>&1 | FileCheck -check-prefix=ERROR %s
 
-; ERROR: error: scalar registers limit of 104 exceeded (106) in use_too_many_sgprs_tahiti
+; ERROR: error: scalar registers (106) exceeds limit (104) in function 'use_too_many_sgprs_tahiti'
 define amdgpu_kernel void @use_too_many_sgprs_tahiti() #0 {
   call void asm sideeffect "", "~{s[0:7]}" ()
   call void asm sideeffect "", "~{s[8:15]}" ()
   call void asm sideeffect "", "~{s[16:23]}" ()
   call void asm sideeffect "", "~{s[24:31]}" ()
   call void asm sideeffect "", "~{s[32:39]}" ()
   call void asm sideeffect "", "~{s[40:47]}" ()
   call void asm sideeffect "", "~{s[48:55]}" ()
   call void asm sideeffect "", "~{s[56:63]}" ()
   call void asm sideeffect "", "~{s[64:71]}" ()
   call void asm sideeffect "", "~{s[72:79]}" ()
   call void asm sideeffect "", "~{s[80:87]}" ()
   call void asm sideeffect "", "~{s[88:95]}" ()
   call void asm sideeffect "", "~{s[96:103]}" ()
   call void asm sideeffect "", "~{vcc}" ()
   ret void
 }
 
-; ERROR: error: scalar registers limit of 104 exceeded (106) in use_too_many_sgprs_bonaire
+; ERROR: error: scalar registers (106) exceeds limit (104) in function 'use_too_many_sgprs_bonaire'
 define amdgpu_kernel void @use_too_many_sgprs_bonaire() #1 {
   call void asm sideeffect "", "~{s[0:7]}" ()
   call void asm sideeffect "", "~{s[8:15]}" ()
   call void asm sideeffect "", "~{s[16:23]}" ()
   call void asm sideeffect "", "~{s[24:31]}" ()
   call void asm sideeffect "", "~{s[32:39]}" ()
   call void asm sideeffect "", "~{s[40:47]}" ()
   call void asm sideeffect "", "~{s[48:55]}" ()
   call void asm sideeffect "", "~{s[56:63]}" ()
   call void asm sideeffect "", "~{s[64:71]}" ()
   call void asm sideeffect "", "~{s[72:79]}" ()
   call void asm sideeffect "", "~{s[80:87]}" ()
   call void asm sideeffect "", "~{s[88:95]}" ()
   call void asm sideeffect "", "~{s[96:103]}" ()
   call void asm sideeffect "", "~{vcc}" ()
   ret void
 }
 
-; ERROR: error: scalar registers limit of 104 exceeded (108) in use_too_many_sgprs_bonaire_flat_scr
+; ERROR: error: scalar registers (108) exceeds limit (104) in function 'use_too_many_sgprs_bonaire_flat_scr'
 define amdgpu_kernel void @use_too_many_sgprs_bonaire_flat_scr() #1 {
   call void asm sideeffect "", "~{s[0:7]}" ()
   call void asm sideeffect "", "~{s[8:15]}" ()
   call void asm sideeffect "", "~{s[16:23]}" ()
   call void asm sideeffect "", "~{s[24:31]}" ()
   call void asm sideeffect "", "~{s[32:39]}" ()
   call void asm sideeffect "", "~{s[40:47]}" ()
   call void asm sideeffect "", "~{s[48:55]}" ()
   call void asm sideeffect "", "~{s[56:63]}" ()
   call void asm sideeffect "", "~{s[64:71]}" ()
   call void asm sideeffect "", "~{s[72:79]}" ()
   call void asm sideeffect "", "~{s[80:87]}" ()
   call void asm sideeffect "", "~{s[88:95]}" ()
   call void asm sideeffect "", "~{s[96:103]}" ()
   call void asm sideeffect "", "~{vcc}" ()
   call void asm sideeffect "", "~{flat_scratch}" ()
   ret void
 }
 
-; ERROR: error: scalar registers limit of 96 exceeded (98) in use_too_many_sgprs_iceland
+; ERROR: error: scalar registers (98) exceeds limit (96) in function 'use_too_many_sgprs_iceland'
 define amdgpu_kernel void @use_too_many_sgprs_iceland() #2 {
   call void asm sideeffect "", "~{vcc}" ()
   call void asm sideeffect "", "~{s[0:7]}" ()
   call void asm sideeffect "", "~{s[8:15]}" ()
   call void asm sideeffect "", "~{s[16:23]}" ()
   call void asm sideeffect "", "~{s[24:31]}" ()
   call void asm sideeffect "", "~{s[32:39]}" ()
   call void asm sideeffect "", "~{s[40:47]}" ()
   call void asm sideeffect "", "~{s[48:55]}" ()
   call void asm sideeffect "", "~{s[56:63]}" ()
   call void asm sideeffect "", "~{s[64:71]}" ()
   call void asm sideeffect "", "~{s[72:79]}" ()
   call void asm sideeffect "", "~{s[80:87]}" ()
   call void asm sideeffect "", "~{s[88:95]}" ()
   ret void
 }
 
-; ERROR: error: addressable scalar registers limit of 102 exceeded (103) in use_too_many_sgprs_fiji
+; ERROR: error: addressable scalar registers (103) exceeds limit (102) in function 'use_too_many_sgprs_fiji'
 define amdgpu_kernel void @use_too_many_sgprs_fiji() #3 {
   call void asm sideeffect "", "~{s[0:7]}" ()
   call void asm sideeffect "", "~{s[8:15]}" ()
   call void asm sideeffect "", "~{s[16:23]}" ()
   call void asm sideeffect "", "~{s[24:31]}" ()
   call void asm sideeffect "", "~{s[32:39]}" ()
   call void asm sideeffect "", "~{s[40:47]}" ()
   call void asm sideeffect "", "~{s[48:55]}" ()

llvm/test/CodeGen/AMDGPU/stack-size-overflow.ll

 ; RUN: not llc -march=amdgcn < %s 2>&1 | FileCheck -check-prefix=ERROR %s
 ; RUN: not llc -march=amdgcn < %s | FileCheck -check-prefix=GCN %s
 
 declare void @llvm.memset.p5i8.i32(i8 addrspace(5)* nocapture, i8, i32, i32, i1) #1
 
-; ERROR: error: stack size limit exceeded (131061) in stack_size_limit_wave64
+; ERROR: error: stack frame size (131061) exceeds limit in function 'stack_size_limit_wave64'
 ; GCN: ; ScratchSize: 131061
 define amdgpu_kernel void @stack_size_limit_wave64() #0 {
 entry:
   %alloca = alloca [131057 x i8], align 1, addrspace(5)
   %alloca.bc = bitcast [131057 x i8] addrspace(5)* %alloca to i8 addrspace(5)*
   call void @llvm.memset.p5i8.i32(i8 addrspace(5)* %alloca.bc, i8 9, i32 131057, i32 1, i1 true)
   ret void
 }
 
-; ERROR: error: stack size limit exceeded (262117) in stack_size_limit_wave32
+; ERROR: error: stack frame size (262117) exceeds limit in function 'stack_size_limit_wave32'
 ; GCN: ; ScratchSize: 262117
 define amdgpu_kernel void @stack_size_limit_wave32() #1 {
 entry:
   %alloca = alloca [262113 x i8], align 1, addrspace(5)
   %alloca.bc = bitcast [262113 x i8] addrspace(5)* %alloca to i8 addrspace(5)*
   call void @llvm.memset.p5i8.i32(i8 addrspace(5)* %alloca.bc, i8 9, i32 262113, i32 1, i1 true)
   ret void
 }

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 size exceeds the
+; 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
   ret void
 }
 
-; CHECK: warning: stack size limit exceeded (92) in warn
+; CHECK: warning: stack frame size (92) exceeds limit (80) in function 'warn'
 define void @warn() nounwind ssp "frame-pointer"="all" "warn-stack-size"="80" {
 entry:
   %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
 }
 
 declare void @doit(i8*)

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 size exceeds the
+; 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
   %arraydecay = getelementptr inbounds [12 x i8], [12 x i8]* %buffer, i64 0, i64 0
   call void @doit(i8* %arraydecay) nounwind
   ret void
 }
 
-; CHECK: warning: stack size limit exceeded (88) in warn
+; CHECK: warning: stack frame size (88) exceeds limit (80) in function 'warn'
 define void @warn() nounwind ssp "warn-stack-size"="80" {
 entry:
   %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
 }
 
 declare void @doit(i8*)