Skip to content

Commit 6861d93

Browse files
LebedevRILebedevRI
committed
Revert "clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM"
See discussion in https://bugs.llvm.org/show_bug.cgi?id=45073 / https://reviews.llvm.org/D66324#2334485 the implementation is known-broken for certain inputs, the bugreport was up for a significant amount of timer, and there has been no activity to address it. Therefore, just completely rip out all of misexpect handling. I suspect, fixing it requires redesigning the internals of MD_misexpect. Should anyone commit to fixing the implementation problem, starting from clean slate may be better anyways. This reverts commit 7bdad08, and some of it's follow-ups, that don't stand on their own.
1 parent e51631c commit 6861d93

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+44
-1867
lines changed

‎clang/docs/DiagnosticsReference.rst

-9
Original file line numberDiff line numberDiff line change
@@ -7977,15 +7977,6 @@ This diagnostic is enabled by default.
79777977
+------------------------------------------------------------------------------------------------+
79787978

79797979

7980-
-Wmisexpect
7981-
-----------
7982-
**Diagnostic text:**
7983-
7984-
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
7985-
|:warning:`warning:` |nbsp| :diagtext:`Potential performance regression from use of \_\_builtin\_expect(): Annotation was correct on` |nbsp| :placeholder:`A` |nbsp| :diagtext:`of profiled executions.`|
7986-
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
7987-
7988-
79897980
-Wmisleading-indentation
79907981
------------------------
79917982
**Diagnostic text:**

‎clang/include/clang/Basic/DiagnosticFrontendKinds.td

-6
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,6 @@ def warn_profile_data_missing : Warning<
299299
def warn_profile_data_unprofiled : Warning<
300300
"no profile data available for file \"%0\"">,
301301
InGroup<ProfileInstrUnprofiled>;
302-
def warn_profile_data_misexpect : Warning<
303-
"Potential performance regression from use of __builtin_expect(): "
304-
"Annotation was correct on %0 of profiled executions.">,
305-
BackendInfo,
306-
InGroup<MisExpect>,
307-
DefaultIgnore;
308302
} // end of instrumentation issue category
309303

310304
}

‎clang/include/clang/Basic/DiagnosticGroups.td

-1
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,6 @@ def BackendOptimizationFailure : DiagGroup<"pass-failed">;
11561156
def ProfileInstrMissing : DiagGroup<"profile-instr-missing">;
11571157
def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
11581158
def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
1159-
def MisExpect : DiagGroup<"misexpect">;
11601159

11611160
// AddressSanitizer frontend instrumentation remarks.
11621161
def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;

‎clang/lib/CodeGen/CodeGenAction.cpp

-36
Original file line numberDiff line numberDiff line change
@@ -404,9 +404,6 @@ namespace clang {
404404
bool StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D);
405405
/// Specialized handler for unsupported backend feature diagnostic.
406406
void UnsupportedDiagHandler(const llvm::DiagnosticInfoUnsupported &D);
407-
/// Specialized handler for misexpect warnings.
408-
/// Note that misexpect remarks are emitted through ORE
409-
void MisExpectDiagHandler(const llvm::DiagnosticInfoMisExpect &D);
410407
/// Specialized handlers for optimization remarks.
411408
/// Note that these handlers only accept remarks and they always handle
412409
/// them.
@@ -674,36 +671,6 @@ void BackendConsumer::UnsupportedDiagHandler(
674671
<< Filename << Line << Column;
675672
}
676673

677-
void BackendConsumer::MisExpectDiagHandler(
678-
const llvm::DiagnosticInfoMisExpect &D) {
679-
StringRef Filename;
680-
unsigned Line, Column;
681-
bool BadDebugInfo = false;
682-
FullSourceLoc Loc;
683-
std::string Msg;
684-
raw_string_ostream MsgStream(Msg);
685-
DiagnosticPrinterRawOStream DP(MsgStream);
686-
687-
// Context will be nullptr for IR input files, we will construct the diag
688-
// message from llvm::DiagnosticInfoMisExpect.
689-
if (Context != nullptr) {
690-
Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
691-
MsgStream << D.getMsg();
692-
} else {
693-
DiagnosticPrinterRawOStream DP(MsgStream);
694-
D.print(DP);
695-
}
696-
Diags.Report(Loc, diag::warn_profile_data_misexpect) << MsgStream.str();
697-
698-
if (BadDebugInfo)
699-
// If we were not able to translate the file:line:col information
700-
// back to a SourceLocation, at least emit a note stating that
701-
// we could not translate this location. This can happen in the
702-
// case of #line directives.
703-
Diags.Report(Loc, diag::note_fe_backend_invalid_loc)
704-
<< Filename << Line << Column;
705-
}
706-
707674
void BackendConsumer::EmitOptimizationMessage(
708675
const llvm::DiagnosticInfoOptimizationBase &D, unsigned DiagID) {
709676
// We only support warnings and remarks.
@@ -881,9 +848,6 @@ void BackendConsumer::DiagnosticHandlerImpl(const DiagnosticInfo &DI) {
881848
case llvm::DK_Unsupported:
882849
UnsupportedDiagHandler(cast<DiagnosticInfoUnsupported>(DI));
883850
return;
884-
case llvm::DK_MisExpect:
885-
MisExpectDiagHandler(cast<DiagnosticInfoMisExpect>(DI));
886-
return;
887851
default:
888852
// Plugin IDs are not bound to any value as they are set dynamically.
889853
ComputeDiagRemarkID(Severity, backend_plugin, DiagID);

‎clang/lib/Frontend/CompilerInvocation.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -3833,9 +3833,6 @@ bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
38333833
}
38343834
}
38353835

3836-
if (Diags.isIgnored(diag::warn_profile_data_misexpect, SourceLocation()))
3837-
Res.FrontendOpts.LLVMArgs.push_back("-pgo-warn-misexpect");
3838-
38393836
LangOpts.FunctionAlignment =
38403837
getLastArgIntValue(Args, OPT_function_alignment, 0, Diags);
38413838

‎clang/test/CodeGen/thinlto-clang-diagnostic-handler-in-be.c

-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
// RUN: llvm-lto -thinlto -o %t %t1.bo
88
// RUN: %clang -cc1 -O2 -fexperimental-new-pass-manager -x ir %t1.bo -fthinlto-index=%t.thinlto.bc -emit-obj -Rpass-analysis=info 2>&1 | FileCheck %s -check-prefix=CHECK-REMARK
99
// RUN: llvm-profdata merge -o %t2.profdata %S/Inputs/thinlto_expect2.proftext
10-
// RUN: %clang -cc1 -O2 -fexperimental-new-pass-manager -x ir %t1.bo -fthinlto-index=%t.thinlto.bc -fprofile-instrument-use-path=%t2.profdata -emit-obj -Wmisexpect 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING
1110
// RUN: %clang -cc1 -O2 -fexperimental-new-pass-manager -x ir %t1.bo -fthinlto-index=%t.thinlto.bc -fprofile-instrument-use-path=%t2.profdata -emit-obj 2>&1 | FileCheck %s -allow-empty -check-prefix=CHECK-NOWARNING
1211

1312
int sum;
@@ -20,5 +19,4 @@ __attribute__((noinline)) void foo(int m) {
2019
bar();
2120
}
2221
// CHECK-REMARK: remark: {{.*}}.c:
23-
// CHECK-WARNING: warning: Potential performance regression from use of __builtin_expect(): Annotation was correct on {{.*}}.c:{{[0-9]*}}:{{[0-9]*}}: 50.00% (12 / 24) of profiled executions.
2422
// CHECK-NOWARNING-NOT: warning: {{.*}}.c:{{[0-9]*}}:26: 50.00% (12 / 24)

‎clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp

+15-15
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ void ab0(int &i) {
3636
// CHECK: br {{.*}}end{{$}}
3737
// CHECK: br {{.*}}end{{$}}
3838
// CHECK: br {{.*}}end{{$}}
39-
// CHECK: br {{.*}} !prof !10
39+
// CHECK: br {{.*}} !prof !9
4040
if (__builtin_expect(a() && b() && c(), 0)) {
4141
++i;
4242
} else {
@@ -48,7 +48,7 @@ void au(int &i) {
4848
// CHECK-LABEL: define{{.*}}au
4949
// CHECK: br {{.*}}else{{$}}
5050
// CHECK: br {{.*}}else{{$}}
51-
// CHECK: br {{.*}} !prof !10
51+
// CHECK: br {{.*}} !prof !9
5252
if (a() && b() && c()) [[unlikely]] {
5353
++i;
5454
} else {
@@ -83,9 +83,9 @@ void ol(int &i) {
8383

8484
void ob0(int &i) {
8585
// CHECK-LABEL: define{{.*}}ob0
86-
// CHECK: br {{.*}} !prof !10
87-
// CHECK: br {{.*}} !prof !10
88-
// CHECK: br {{.*}} !prof !10
86+
// CHECK: br {{.*}} !prof !9
87+
// CHECK: br {{.*}} !prof !9
88+
// CHECK: br {{.*}} !prof !9
8989
if (__builtin_expect(a() || b() || c(), 0)) {
9090
i = 0;
9191
} else {
@@ -95,9 +95,9 @@ void ob0(int &i) {
9595

9696
void ou(int &i) {
9797
// CHECK-LABEL: define{{.*}}ou
98-
// CHECK: br {{.*}} !prof !10
99-
// CHECK: br {{.*}} !prof !10
100-
// CHECK: br {{.*}} !prof !10
98+
// CHECK: br {{.*}} !prof !9
99+
// CHECK: br {{.*}} !prof !9
100+
// CHECK: br {{.*}} !prof !9
101101
if (a() || b() || c()) [[unlikely]] {
102102
i = 0;
103103
} else {
@@ -127,7 +127,7 @@ void nl(int &i) {
127127

128128
void nb0(int &i) {
129129
// CHECK-LABEL: define{{.*}}nb0
130-
// CHECK: br {{.*}} !prof !10
130+
// CHECK: br {{.*}} !prof !9
131131
if (__builtin_expect(!a(), 0)) {
132132
++i;
133133
} else {
@@ -137,7 +137,7 @@ void nb0(int &i) {
137137

138138
void nu(int &i) {
139139
// CHECK-LABEL: define{{.*}}nu
140-
// CHECK: br {{.*}} !prof !10
140+
// CHECK: br {{.*}} !prof !9
141141
if (bool d = !a()) [[unlikely]] {
142142
++i;
143143
} else {
@@ -188,7 +188,7 @@ void tb0(int &i) {
188188
// CHECK: br {{.*}}false{{$}}
189189
// CHECK: br {{.*}}end{{$}}
190190
// CHECK: br {{.*}}end{{$}}
191-
// CHECK: br {{.*}} !prof !10
191+
// CHECK: br {{.*}} !prof !9
192192
if (__builtin_expect(a() ? b() : c(), 0)) {
193193
++i;
194194
} else {
@@ -201,7 +201,7 @@ void tu(int &i) {
201201
// CHECK: br {{.*}}false{{$}}
202202
// CHECK: br {{.*}}end{{$}}
203203
// CHECK: br {{.*}}end{{$}}
204-
// CHECK: br {{.*}} !prof !10
204+
// CHECK: br {{.*}} !prof !9
205205
if (bool d = a() ? b() : c()) [[unlikely]] {
206206
++i;
207207
} else {
@@ -212,8 +212,8 @@ void tu(int &i) {
212212
void tu2(int &i) {
213213
// CHECK-LABEL: define{{.*}}tu
214214
// CHECK: br {{.*}}false{{$}}
215-
// CHECK: br {{.*}} !prof !10
216-
// CHECK: br {{.*}} !prof !10
215+
// CHECK: br {{.*}} !prof !9
216+
// CHECK: br {{.*}} !prof !9
217217
if (a() ? b() : c()) [[unlikely]] {
218218
++i;
219219
} else {
@@ -222,4 +222,4 @@ void tu2(int &i) {
222222
}
223223

224224
// CHECK: !6 = !{!"branch_weights", i32 2000, i32 1}
225-
// CHECK: !10 = !{!"branch_weights", i32 1, i32 2000}
225+
// CHECK: !9 = !{!"branch_weights", i32 1, i32 2000}

‎clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext

-9
This file was deleted.

‎clang/test/Profile/Inputs/misexpect-branch.proftext

-9
This file was deleted.

‎clang/test/Profile/Inputs/misexpect-switch-default-only.proftext

-12
This file was deleted.

‎clang/test/Profile/Inputs/misexpect-switch-default.proftext

-16
This file was deleted.

‎clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext

-17
This file was deleted.

‎clang/test/Profile/Inputs/misexpect-switch.proftext

-16
This file was deleted.

‎clang/test/Profile/misexpect-branch-cold.c

-26
This file was deleted.

‎clang/test/Profile/misexpect-branch-nonconst-expected-val.c

-23
This file was deleted.

‎clang/test/Profile/misexpect-branch-unpredictable.c

-25
This file was deleted.

0 commit comments

Comments
 (0)