[llvm] [SampleProfileLoader] Fix integer overflow in generateMDProfMetadata (PR #90217)
Nabeel Omer via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 26 08:52:43 PDT 2024
https://github.com/omern1 updated https://github.com/llvm/llvm-project/pull/90217
>From 14d695d44454638775ec3903a8ed7ace112685aa Mon Sep 17 00:00:00 2001
From: Nabeel Omer <Nabeel.Omer at sony.com>
Date: Fri, 26 Apr 2024 14:12:17 +0100
Subject: [PATCH 1/4] Add pre-commit tests
---
.../SampleProfile/Inputs/overflow.proftext | 2 +
.../test/Transforms/SampleProfile/overflow.ll | 70 +++++++++++++++++++
2 files changed, 72 insertions(+)
create mode 100644 llvm/test/Transforms/SampleProfile/Inputs/overflow.proftext
create mode 100644 llvm/test/Transforms/SampleProfile/overflow.ll
diff --git a/llvm/test/Transforms/SampleProfile/Inputs/overflow.proftext b/llvm/test/Transforms/SampleProfile/Inputs/overflow.proftext
new file mode 100644
index 00000000000000..753294a49e99c5
--- /dev/null
+++ b/llvm/test/Transforms/SampleProfile/Inputs/overflow.proftext
@@ -0,0 +1,2 @@
+_Z3testi:29600000000:29600000000
+ 5: 29600000000
diff --git a/llvm/test/Transforms/SampleProfile/overflow.ll b/llvm/test/Transforms/SampleProfile/overflow.ll
new file mode 100644
index 00000000000000..67546f46dd986b
--- /dev/null
+++ b/llvm/test/Transforms/SampleProfile/overflow.ll
@@ -0,0 +1,70 @@
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/overflow.proftext | opt -passes='print<branch-prob>' -disable-output 2>&1 | FileCheck %s
+
+; Original Source:
+; int sqrt(int);
+; int lol(int i) {
+; if (i == 5) {
+; return 42;
+; }
+; else {
+; return sqrt(i);
+; }
+;}
+
+define dso_local noundef i32 @_Z3testi(i32 noundef %i) local_unnamed_addr #0 !dbg !10 {
+entry:
+ tail call void @llvm.dbg.value(metadata i32 %i, metadata !16, metadata !DIExpression()), !dbg !17
+ %cmp = icmp eq i32 %i, 5, !dbg !18
+ br i1 %cmp, label %return, label %if.else, !dbg !20
+
+if.else: ; preds = %entry
+ %call = tail call noundef i32 @_Z4sqrti(i32 noundef %i), !dbg !21
+ br label %return, !dbg !23
+
+return: ; preds = %entry, %if.else
+ %retval.0 = phi i32 [ %call, %if.else ], [ 42, %entry ], !dbg !24
+ ret i32 %retval.0, !dbg !25
+}
+
+declare !dbg !26 noundef i32 @_Z4sqrti(i32 noundef)
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+attributes #0 = { "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "/", checksumkind: CSK_MD5, checksum: "cb38d90153a7ebdd6ecf3058eb0524c7")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
+!9 = !{!"clang"}
+!10 = distinct !DISubprogram(name: "test", linkageName: "_Z3loli", scope: !11, file: !11, line: 3, type: !12, scopeLine: 3, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !15)
+!11 = !DIFile(filename: "./test.cpp", directory: "/", checksumkind: CSK_MD5, checksum: "cb38d90153a7ebdd6ecf3058eb0524c7")
+!12 = !DISubroutineType(types: !13)
+!13 = !{!14, !14}
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!15 = !{!16}
+!16 = !DILocalVariable(name: "i", arg: 1, scope: !10, file: !11, line: 3, type: !14)
+!17 = !DILocation(line: 0, scope: !10)
+!18 = !DILocation(line: 4, column: 11, scope: !19)
+!19 = distinct !DILexicalBlock(scope: !10, file: !11, line: 4, column: 9)
+!20 = !DILocation(line: 4, column: 9, scope: !10)
+!21 = !DILocation(line: 8, column: 16, scope: !22)
+!22 = distinct !DILexicalBlock(scope: !19, file: !11, line: 7, column: 10)
+!23 = !DILocation(line: 8, column: 9, scope: !22)
+!24 = !DILocation(line: 0, scope: !19)
+!25 = !DILocation(line: 10, column: 1, scope: !10)
+!26 = !DISubprogram(name: "sqrt", linkageName: "_Z4sqrti", scope: !11, file: !11, line: 1, type: !12, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+
+; CHECK: edge %entry -> %return probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge %entry -> %if.else probability is 0x00000000 / 0x80000000 = 0.00%
+; CHECK: edge %if.else -> %return probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
>From e08dfb3c606494a19763b53ccd60993172643a37 Mon Sep 17 00:00:00 2001
From: Nabeel Omer <Nabeel.Omer at sony.com>
Date: Fri, 26 Apr 2024 14:13:36 +0100
Subject: [PATCH 2/4] [SampleProfileLoader] Fix integer overflow in
generateMDProfMetadata
This patch fixes an integer overflow in the SampleProfileLoader pass.
The issue occurs when weights are saturated and Profi isn't being used.
This patch also adds a newline to a debug message to make it more
readable.
---
llvm/lib/Transforms/IPO/SampleProfile.cpp | 7 +++++--
llvm/test/Transforms/SampleProfile/overflow.ll | 4 ++--
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index 0b3a6931e779b6..7018e3feded8ab 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1713,13 +1713,16 @@ void SampleProfileLoader::generateMDProfMetadata(Function &F) {
// if needed. Sample counts in profiles are 64-bit unsigned values,
// but internally branch weights are expressed as 32-bit values.
if (Weight > std::numeric_limits<uint32_t>::max()) {
- LLVM_DEBUG(dbgs() << " (saturated due to uint32_t overflow)");
+ LLVM_DEBUG(dbgs() << " (saturated due to uint32_t overflow)\n");
Weight = std::numeric_limits<uint32_t>::max();
}
if (!SampleProfileUseProfi) {
// Weight is added by one to avoid propagation errors introduced by
// 0 weights.
- Weights.push_back(static_cast<uint32_t>(Weight + 1));
+ if (Weight != std::numeric_limits<uint32_t>::max())
+ Weight += 1;
+
+ Weights.push_back(static_cast<uint32_t>(Weight));
} else {
// Profi creates proper weights that do not require "+1" adjustments but
// we evenly split the weight among branches with the same destination.
diff --git a/llvm/test/Transforms/SampleProfile/overflow.ll b/llvm/test/Transforms/SampleProfile/overflow.ll
index 67546f46dd986b..76c7ce874093a1 100644
--- a/llvm/test/Transforms/SampleProfile/overflow.ll
+++ b/llvm/test/Transforms/SampleProfile/overflow.ll
@@ -65,6 +65,6 @@ attributes #0 = { "use-sample-profile" }
!25 = !DILocation(line: 10, column: 1, scope: !10)
!26 = !DISubprogram(name: "sqrt", linkageName: "_Z4sqrti", scope: !11, file: !11, line: 1, type: !12, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
-; CHECK: edge %entry -> %return probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
-; CHECK: edge %entry -> %if.else probability is 0x00000000 / 0x80000000 = 0.00%
+; CHECK: edge %entry -> %return probability is 0x00000000 / 0x80000000 = 0.00%
+; CHECK: edge %entry -> %if.else probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
; CHECK: edge %if.else -> %return probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
>From 6e9d7c17a67a0b27c0a828440795793cf2aa33cb Mon Sep 17 00:00:00 2001
From: Nabeel Omer <Nabeel.Omer at sony.com>
Date: Fri, 26 Apr 2024 16:48:07 +0100
Subject: [PATCH 3/4] Fix failing test fnptr.ll
The test is only intended to check whether different profile encoding
formats produce the same profile annotations.
---
llvm/test/Transforms/SampleProfile/fnptr.ll | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/test/Transforms/SampleProfile/fnptr.ll b/llvm/test/Transforms/SampleProfile/fnptr.ll
index 55ee75e3353a48..00fa27081447d0 100644
--- a/llvm/test/Transforms/SampleProfile/fnptr.ll
+++ b/llvm/test/Transforms/SampleProfile/fnptr.ll
@@ -9,8 +9,8 @@
; CHECK: edge %for.body3 -> %if.else probability is 0x65a95a96 / 0x80000000 = 79.42%
; CHECK: edge %for.inc -> %for.inc12 probability is 0x000fbd1c / 0x80000000 = 0.05%
; CHECK: edge %for.inc -> %for.body3 probability is 0x7ff042e4 / 0x80000000 = 99.95%
-; CHECK: edge %for.inc12 -> %for.end14 probability is 0x04000000 / 0x80000000 = 3.12%
-; CHECK: edge %for.inc12 -> %for.cond1.preheader probability is 0x7c000000 / 0x80000000 = 96.88%
+; CHECK: edge %for.inc12 -> %for.end14 probability is 0x40000000 / 0x80000000 = 50.00%
+; CHECK: edge %for.inc12 -> %for.cond1.preheader probability is 0x40000000 / 0x80000000 = 50.00%
; Original C++ test case.
;
>From a9aa67bbbda5e07b150092dc244cb954a224ec34 Mon Sep 17 00:00:00 2001
From: Nabeel Omer <Nabeel.Omer at sony.com>
Date: Fri, 26 Apr 2024 16:51:57 +0100
Subject: [PATCH 4/4] Add comment to test explaining what it does
---
llvm/test/Transforms/SampleProfile/overflow.ll | 2 ++
1 file changed, 2 insertions(+)
diff --git a/llvm/test/Transforms/SampleProfile/overflow.ll b/llvm/test/Transforms/SampleProfile/overflow.ll
index 76c7ce874093a1..23c6fbc0dc1791 100644
--- a/llvm/test/Transforms/SampleProfile/overflow.ll
+++ b/llvm/test/Transforms/SampleProfile/overflow.ll
@@ -1,3 +1,5 @@
+; Checks that we are able to handle overflowing counters correctly.
+
; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/overflow.proftext | opt -passes='print<branch-prob>' -disable-output 2>&1 | FileCheck %s
; Original Source:
More information about the llvm-commits
mailing list