[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D115907#4296154 , @hans wrote:

> In D115907#4295923 , @paulkirth 
> wrote:
>
>>> 2. Due to inlining etc., it often gets the source locations wrong, which 
>>> means it points at code where again there were no expectations -- but 
>>> perhaps that code got inlined into an expectations somewhere else. (e.g. 
>>> https://crbug.com/1434989#c9)
>>
>> The checking depends somewhat on the instrumentation type (frontend vs. 
>> backend instrumentation) In the case of frontend instrumentation, when the 
>> expect intrinsic is lowered (very early in the pipeline) we can report the 
>> diagnostic right away, since branch weights from profiling have already been 
>> attached. The //should// mean that you should get fairly accurate source 
>> information, since this happens before any optimizations run.
>
> We use the backend instrumentation since that gives the best performance, but 
> it's good to hear that the locations may be more accurate with frontend 
> instrumentation. Maybe we should add that to the docs?

Let me look into this a bit. I'm not sure if this is just we're not doing the 
best practice when printing the diagnostic, or if the resolution is being 
degraded due to optimizations. We may want to update the documentation anyway, 
but I'd like to be certain.




Comment at: clang/include/clang/Driver/Options.td:1434
+def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], 
"fdiagnostics-misexpect-tolerance=">,
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent misexpect diagnostics from being output if the profile 
counts are within N% of the expected. ">;

hans wrote:
> paulkirth wrote:
> > hans wrote:
> > > Should this be a driver mode option and not just cc1? The doc suggests 
> > > using it, which currently won't work without an `-Xclang` prefix.
> > That should also work from clang w/o `-Xclang`, shouldn't it? This is used 
> > exactly like `-fdiagnostics-hotness-threshold=`, so IIRC that should still 
> > work from clang.
> Hmm, if I add `-fdiagnostics-misexpect-tolerance=10` I get errors about the 
> flag being unused. I had to use `-Xclang` for it to work.
That isn't WAI then. I'll try to address that today.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D115907#4295923 , @paulkirth wrote:

>> 2. Due to inlining etc., it often gets the source locations wrong, which 
>> means it points at code where again there were no expectations -- but 
>> perhaps that code got inlined into an expectations somewhere else. (e.g. 
>> https://crbug.com/1434989#c9)
>
> The checking depends somewhat on the instrumentation type (frontend vs. 
> backend instrumentation) In the case of frontend instrumentation, when the 
> expect intrinsic is lowered (very early in the pipeline) we can report the 
> diagnostic right away, since branch weights from profiling have already been 
> attached. The //should// mean that you should get fairly accurate source 
> information, since this happens before any optimizations run.

We use the backend instrumentation since that gives the best performance, but 
it's good to hear that the locations may be more accurate with frontend 
instrumentation. Maybe we should add that to the docs?

> The log also reported several places where expected hot code was never 
> executed. If that isn't desirable, I'd also suggest that you could use the 
> `-fdiagnostic-hotness-threshold=` to ignore reporting about branches that are 
> not executed some minimum number of times. MisExpect is remarks based, so I 
> beleive that is currently working. If not I'm happy to add that functionality.

Oh nice, I will try that flag.




Comment at: clang/include/clang/Driver/Options.td:1434
+def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], 
"fdiagnostics-misexpect-tolerance=">,
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent misexpect diagnostics from being output if the profile 
counts are within N% of the expected. ">;

paulkirth wrote:
> hans wrote:
> > Should this be a driver mode option and not just cc1? The doc suggests 
> > using it, which currently won't work without an `-Xclang` prefix.
> That should also work from clang w/o `-Xclang`, shouldn't it? This is used 
> exactly like `-fdiagnostics-hotness-threshold=`, so IIRC that should still 
> work from clang.
Hmm, if I add `-fdiagnostics-misexpect-tolerance=10` I get errors about the 
flag being unused. I had to use `-Xclang` for it to work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

Thanks for the report Hans.

In D115907#4295363 , @hans wrote:

> We gave this a try in Chromium: 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1434989
>
> While the diagnostics are interesting, two things make this less useful for 
> our developers:
>
> 1. It also fires in situations where the developer didn't set any 
> expectations, for example for static variable initialization guards where 
> clang implicitly adds branch weights (https://crbug.com/1434989#c7)

Hmm, I wasn't even aware that clang would insert `llvm.expect` without a user 
annotation. I'm also a bit surprised to see this crop up, since we haven't run 
into it on Fuchsia.

The problem is that we can't really distinguish those cases in IR... and even 
when we could, for IR based instrumentation we'd need to embed that into the 
branch weights somehow. D131306  explores a 
way to track the provenance of branch weights, so we could probably use that to 
distinguish them if we could also distinguish the `llvm.expect` usages the 
compiler inserts from user inserted ones. I put that work on hold, since it 
needs an RFC and I've been busy w/ other things, but I'll take some time in the 
next day or two to write that out and post the RFC to discord, since D131306 
 should solve a big part of that issue.

> 2. Due to inlining etc., it often gets the source locations wrong, which 
> means it points at code where again there were no expectations -- but perhaps 
> that code got inlined into an expectations somewhere else. (e.g. 
> https://crbug.com/1434989#c9)

The checking depends somewhat on the instrumentation type (frontend vs. backend 
instrumentation) In the case of frontend instrumentation, when the expect 
intrinsic is lowered (very early in the pipeline) we can report the diagnostic 
right away, since branch weights from profiling have already been attached. The 
//should// mean that you should get fairly accurate source information, since 
this happens before any optimizations run.

Backend instrumentation is more complicated, since we can't report the 
discrepancy until we're adding the branch weights to the IR. Whether inlining 
plays a role there or not is highly dependent on how you're compiling (i.e., no 
LTO, LTO, ThinLTO). The ordering is also somewhat dependent on if you're using 
Sampling or Instrumentation based profiles, but I believe both of those will 
always add weights before inlining. ThinLTO may be an outlier here, since I 
think weights from sampling based profiles can run multiple times in the 
ThinLTO pipeline.

> Especially 2) is probably not easy to fix. Do you have any tips on how 
> developers can use this more effectively?

When we report the diagnostic, we do our best to provide the source location, 
but that is only as good as what is available in the IR. I can certainly take a 
look at how we could improve reporting, but my recollection is that source 
information isn't always maintained well as the various passes run.If we're 
lucky in this case, we can maybe just tweak how its reporting the source 
location to include the inlining context. There's probably a bigger discussion 
that needs to be had about how to preserve debug and source location info 
through the backend passes.

The log also reported several places where expected hot code was never 
executed. If that isn't desirable, I'd also suggest that you could use the 
`-fdiagnostic-hotness-threshold=` to ignore reporting about branches that are 
not executed some minimum number of times. MisExpect is remarks based, so I 
beleive that is currently working. If not I'm happy to add that functionality.




Comment at: clang/include/clang/Driver/Options.td:1434
+def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], 
"fdiagnostics-misexpect-tolerance=">,
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent misexpect diagnostics from being output if the profile 
counts are within N% of the expected. ">;

hans wrote:
> Should this be a driver mode option and not just cc1? The doc suggests using 
> it, which currently won't work without an `-Xclang` prefix.
That should also work from clang w/o `-Xclang`, shouldn't it? This is used 
exactly like `-fdiagnostics-hotness-threshold=`, so IIRC that should still work 
from clang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.
Herald added subscribers: hoy, Enna1, StephenFan.

We gave this a try in Chromium: 
https://bugs.chromium.org/p/chromium/issues/detail?id=1434989

While the diagnostics are interesting, two things make this less useful for our 
developers:

1. It also fires in situations where the developer didn't set any expectations, 
for example for static variable initialization guards where clang implicitly 
adds branch weights (https://crbug.com/1434989#c7)
2. Due to inlining etc., it often gets the source locations wrong, which means 
it points at code where again there were no expectations -- but perhaps that 
code got inlined into an expectations somewhere else. (e.g. 
https://crbug.com/1434989#c9)

Especially 2) is probably not easy to fix. Do you have any tips on how 
developers can use this more effectively?




Comment at: clang/include/clang/Driver/Options.td:1434
+def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], 
"fdiagnostics-misexpect-tolerance=">,
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent misexpect diagnostics from being output if the profile 
counts are within N% of the expected. ">;

Should this be a driver mode option and not just cc1? The doc suggests using 
it, which currently won't work without an `-Xclang` prefix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

Ah, thanks. I've had to try to re-land this so many times, I wanted to be sure 
the pre-submit was looking OK after rebasing. or at least be sure it didn't 
look like it was failing from any of my changes.

w.r.t. `clamp`, keeping it compatible was my intent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

lgtm if you are waiting for a re-review. As noted in my previous comment, you 
have added a little bit of unnecessary checking at the lower end with your new 
clamp function, since it is an unsigned value and can never go below 0. But it 
isn't a big deal if you prefer to keep that in to make it easier to transition 
to std::clamp.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150
+// TODO: when clang allows c++17, use std::clamp instead
+uint32_t clamp(uint64_t value, uint32_t hi, uint32_t low) {
+  if (value > hi)

Nit: seems more intuitive to pass low before high. But not sure the low is 
needed in this case, see comment at callsite.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:201
+  auto Tolerance = getMisExpectTolerance(I.getContext());
+  Tolerance = clamp(Tolerance, 99, 0);
+

Looks like Tolerance is unsigned, as are the arguments to clamp. So it can 
never go below 0.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

@jgorbe Sorry for the trouble. Thank you for the backtrace and the revert. 
Indeed, it seems like I've missed an assumption w.r.t.  over/underflow, and 
will have to sort that out before re-landing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Jorge Gorbe Moya via Phabricator via cfe-commits]

jgorbe added a comment.

By the way, the line number for `llvm::misexpect::verifyMisExpect` in the stack 
trace above includes the debug `printf`s I inserted to get the variable values 
so it won't match the exact line number in the repo. But I think that's the 
only call to `BranchProbability` in that function so I hope it's not very 
confusing anyway. Sorry about that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Jorge Gorbe Moya via Phabricator via cfe-commits]

jgorbe added a comment.

Hi, this patch is causing floating point exceptions for us during profile 
generation. On a debug build I get this assertion failure (see stack trace 
below):

  clang: 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/BranchProbability.cpp:41:
 llvm::BranchProbability::BranchProbability(uint32_t, uint32_t): Assertion 
`Denominator > 0 && "Denominator cannot be 0!"' failed.

Printing some values around the problem I got

  TotalBranchWeight = 4294967296   
  LikelyBranchWeight = 2147483648   

   
  UnlikelyBranchWeight = 2147483648 

   
  NumUnlikelyTargets = 1

I see the `BranchProbability` constructor takes `uint32_t`s, so this looks like 
it's overflowing to 0 (4294967296 is exactly 2**32).

I'm going to revert the patch to unbreak our build. Please let me know if you 
need more details and I'll try to come up with a reproducer I can share. Here's 
the stack trace for the assertion.

   #0 0x0a7f992a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:565:11
   #1 0x0a7f9afb PrintStackTraceSignalHandler(void*) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:632:1 
  
   #2 0x0a7f80bb llvm::sys::RunSignalHandlers() 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Signals.cpp:102:5  
   
   #3 0x0a7fa271 SignalHandler(int) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/Unix/Signals.inc:407:1 

   
   #4 0x7ff57cfe2200 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x13200) 

 
   #5 0x7ff57ca678a1 raise ./signal/../sysdeps/unix/sysv/linux/raise.c:50:1 

   
   #6 0x7ff57ca51546 abort ./stdlib/abort.c:81:7

   
   #7 0x7ff57ca5142f get_sysdep_segment_value ./intl/loadmsgcat.c:509:8 

   
   #8 0x7ff57ca5142f _nl_load_domain ./intl/loadmsgcat.c:970:34 

   
   #9 0x7ff57ca60222 (/lib/x86_64-linux-gnu/libc.so.6+0x35222)  

   
  #10 0x0a6cb517 llvm::BranchProbability::BranchProbability(unsigned 
int, unsigned int) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Support/BranchProbability.cpp:0:3
 
  #11 0x0a937a4d llvm::misexpect::verifyMisExpect(llvm::Instruction&, 
llvm::ArrayRef, llvm::ArrayRef) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:190:54



  #12 0x0a937ef3 
llvm::misexpect::checkBackendInstrumentation(llvm::Instruction&, 
llvm::ArrayRef) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:217:1
  
  #13 0x0a93807b 
llvm::misexpect::checkExpectAnnotations(llvm::Instruction&, 
llvm::ArrayRef, bool) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/Utils/MisExpect.cpp:236:1
 
  #14 0x09e1339c (anonymous 
namespace)::SampleProfileLoader::generateMDProfMetadata(llvm::Function&) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:1755:19
 
  #15 0x09e10d94 (anonymous 
namespace)::SampleProfileLoader::emitAnnotations(llvm::Function&) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:0:5

  #16 0x09e1022b (anonymous 
namespace)::SampleProfileLoader::runOnFunction(llvm::Function&, 
llvm::AnalysisManager*) 
/usr/local/google/home/jgorbe/code/llvm/llvm/lib/Transforms/IPO/SampleProfile.cpp:2199:5
  

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG46774df30715: [misexpect] Re-implement MisExpect Diagnostics 
(authored by paulkirth).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/index.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/docs/UserGuides.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 419181.
paulkirth added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/index.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/docs/UserGuides.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; DISABLED-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; DISABLED-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 418924.
paulkirth added a comment.
Herald added a subscriber: arphaman.

Fix missing reference in TOC & fix formatting of tables


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/index.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/docs/UserGuides.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; DISABLED-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

reverted. I will fix this tomorrow. Sorry for the trouble


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Tanya Lattner via Phabricator via cfe-commits]

tonic added a comment.

This is breaking the docs build with the following warning: 
"MisExpect.rst:document isn't included in any toctree." By default, docs 
warnings are treated as errors. Can you fix this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2add3fbd976d: [misexpect] Re-implement MisExpect Diagnostics 
(authored by paulkirth).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; DISABLED-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; DISABLED-NOT: remark: 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D115907#3412685 , @paulkirth wrote:

> @tejohnson thanks for pointing me to the document. I knew it had something to 
> do w/ CC1 but missed that this was well documented.

I was equally unaware of the documentation of this until you asked and I tried 
to come up with a reasonable explanation for you =)

> Is there anything else that needs to be done, or do you think this is good to 
> land again?

lgtm


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

@tejohnson thanks for pointing me to the document. I knew it had something to 
do w/ CC1 but missed that this was well documented.

Is there anything else that needs to be done, or do you think this is good to 
land again?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1554
+  Twine(*Opts.DiagnosticsMisExpectTolerance), SA);
+
   for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeRecover))

paulkirth wrote:
> I really don't understand why this step is necessary, or why in my local 
> builds omitting the call to `GenerateArgs` works at all. I only arrived at 
> this change by noticing other options do the same, and this seemed to fix the 
> issue with tests using the repro instructions from the precommit bots.
> 
> It seems a bit strange that Clang parses the option, stores it to a 
> `CodeGenOption` then puts it back as a string argument to be parsed again 
> later and put into the same data structure. Is this a result of an earlier 
> architecture in Clang? if so, should we reconsider its design?
My understanding is that this is required as part of the cc1 command line 
parsing. Not sure why it worked in your local builds but not elsewhere. There 
is some info here: 
https://clang.llvm.org/docs/InternalsManual.html#adding-new-command-line-option


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 418328.
paulkirth added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; DISABLED-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; DISABLED-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 418280.
paulkirth added a comment.

Fix check for tolerance diagnostic since we now use GenerateArgs the same way 
as hotness threshold.

Since it has a default value, we need to also ensure that it uses a pattern 
similar to other options.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1554
+  Twine(*Opts.DiagnosticsMisExpectTolerance), SA);
+
   for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeRecover))

I really don't understand why this step is necessary, or why in my local builds 
omitting the call to `GenerateArgs` works at all. I only arrived at this change 
by noticing other options do the same, and this seemed to fix the issue with 
tests using the repro instructions from the precommit bots.

It seems a bit strange that Clang parses the option, stores it to a 
`CodeGenOption` then puts it back as a string argument to be parsed again later 
and put into the same data structure. Is this a result of an earlier 
architecture in Clang? if so, should we reconsider its design?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 418025.
paulkirth added a comment.

Add missing GenerateArgs call to propagate flags to the backend outside of cc1


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; DISABLED-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; DISABLED-NOT: remark: misexpect-switch.c:26:30: Potential 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

The test also failed in the Phabricator pre-commit CI, please keep an eye on it 
before re-submitting (failure link for latest diff was 
https://buildkite.com/llvm-project/premerge-checks/builds/83983#39c06525-7452-412d-af83-ae2cc2d30cdc)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

These pass for me locally, so I'm reverting for now and will dig into this 
tomorrow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests: http://45.33.8.238/linux/71396/step_7.txt

Please take a look, and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe7749d4713a5: [misexpect] Re-implement MisExpect Diagnostics 
(authored by paulkirth).

Changed prior to commit:
  https://reviews.llvm.org/D115907?vs=416356=416360#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth marked 5 inline comments as done.
paulkirth added inline comments.



Comment at: llvm/docs/MisExpect.rst:54
+
+.. option:: -pgo-warn-misexpect
+

tejohnson wrote:
> paulkirth wrote:
> > tejohnson wrote:
> > > Looking at the code, the -pgo-warn-misexpect seems to be useful for 
> > > internal LLVM testing via 'opt', to mimic -Wmisexpect, so it probably 
> > > should be documented as such.
> > Yes, its useful to test llvm, but shouldn't `opt` be usable in the same 
> > way? To me it seems useful for opt to support the warning directly too, but 
> > I'm happy to defer here if you think that's confusing or shouldn't be the 
> > case.
> Oh I'm not disagreeing with having the internal option and using it for opt, 
> that's very useful. My comment was in the context of not having the 
> user-facing clang documentation with the clang driver level option (which you 
> since added). I was just suggesting you add a note that this option is an 
> alternative to the clang option for use when e.g. testing via opt. Since 
> there is now separate clang documentation I think it is less important now.
Thanks for the clarification.



Comment at: llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll:3
+
+; RUN: opt < %s -lower-expect -pgo-instr-use 
-pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 
-pass-remarks=misexpect 2>&1 | FileCheck %s
+

tejohnson wrote:
> tejohnson wrote:
> > The old PM is gone in the optimization pipeline, so these old PM lines in 
> > this and other tests and the comment about the new PM can go away.
> I see you removed the NewPM-style invocations and kept the legacy ones - 
> afaict opt will now convert the legacy style pass invocations to the new PM, 
> but I think it is preferable to use the New PM style invocations.
yes, I misread your earlier comment.  I have restored the New PM style opt 
invocations, and removed the legacy PM ones.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 416356.
paulkirth marked 4 inline comments as done.
paulkirth added a comment.

Fixup remaining outstanding issues

- fix typos
- add misexpect to clang release notes
- restore new pm style opt invocations in llvm tests
- move single header function into implementation file
- clarify comment in test file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,285 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Add Clang release note as well please.




Comment at: clang/docs/MisExpect.rst:58
+   Relaxes misexpect checking to tolerate profiling values within N% of the 
+   expected branch weight. e.g., a vlaue of ``N=5`` allows misexpect to check 
against
+   ``0.95 * Threshold``

value


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

lgtm

Just some minor cleanup before submitting as noted below.




Comment at: clang/test/Profile/misexpect-branch.c:3
+
+// test likely branch
+// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata

Comment is a little unclear, as it is testing both likely and unlikely branches.



Comment at: clang/test/Profile/misexpect-branch.c:9
+// there should be no diagnostics when the tolerance is sufficiently high, or 
when -Wmisexpect is not requested
+//
+// RUN: %clang_cc1 %s -O2 -o - -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify=foo 
-fdiagnostics-misexpect-tolerance=10 -Wmisexpect 
-debug-info-kind=line-tables-only

nit: stray empty comment line?



Comment at: clang/test/Profile/misexpect-branch.c:25
+  int x = 0;
+  if (likely(rando % (outer_loop * inner_loop) == 0)) { // exact-warning-re 
{{Potential performance regression from use of __builtin_expect(): Annotation 
was correct on {{.+}}% ({{[0-9]+ / [0-9]+}}) of profiled executions.}}
+x = baz(rando);

paulkirth wrote:
> tejohnson wrote:
> > Can you add a case where an unlikely() is wrong?
> I've added one, but I'm not sure it tests anything meaningfully, since an 
> unlikely path being too hot is really the same check as a likely path being 
> too cold. 
Right, it is just for completeness to test / illustrate that the handling works 
in both directions.



Comment at: llvm/docs/MisExpect.rst:2
+===
+Misexpect
+===

paulkirth wrote:
> tejohnson wrote:
> > paulkirth wrote:
> > > tejohnson wrote:
> > > > I don't see a mention of -Wmisexpect in the document. Should there be 
> > > > user-facing clang documentation, this seems more specific to the LLVM 
> > > > implementation?
> > > I will update the patch to include similar documentation this for clang.
> > > 
> > > Additionally, my understanding is that documentation for warnings is 
> > > auto-generated from the tablegen, so that at least will be available 
> > > automatically.
> > > 
> > > 
> > Should the clang documentation already be added to this patch? I couldn't 
> > find it.
> In the last update the clang documentation was a bit too close to the llvm 
> documentation. I think they are distinct enough now to be separately useful, 
> but maybe it's better to merge the two?
Yeah there is a fair amount of overlap, but I at least would want the clang 
documentation you added. Seems fine to have the llvm internals documentation 
too, though.



Comment at: llvm/docs/MisExpect.rst:54
+
+.. option:: -pgo-warn-misexpect
+

paulkirth wrote:
> tejohnson wrote:
> > Looking at the code, the -pgo-warn-misexpect seems to be useful for 
> > internal LLVM testing via 'opt', to mimic -Wmisexpect, so it probably 
> > should be documented as such.
> Yes, its useful to test llvm, but shouldn't `opt` be usable in the same way? 
> To me it seems useful for opt to support the warning directly too, but I'm 
> happy to defer here if you think that's confusing or shouldn't be the case.
Oh I'm not disagreeing with having the internal option and using it for opt, 
that's very useful. My comment was in the context of not having the user-facing 
clang documentation with the clang driver level option (which you since added). 
I was just suggesting you add a note that this option is an alternative to the 
clang option for use when e.g. testing via opt. Since there is now separate 
clang documentation I think it is less important now.



Comment at: llvm/include/llvm/Transforms/Utils/MisExpectToleranceParser.h:27
+// Valid option values are integers in the range [0, 100)
+inline Expected> parseToleranceOption(StringRef Arg) {
+  int64_t Val;

Given that there is only one callsite to this helper, I think just move it into 
the calling source file and avoid adding a new header.



Comment at: llvm/lib/IR/LLVMContextImpl.h:1384
+  /// The percentage of difference between profiling branch weights and
+  // llvm.expect branch weights to tollerate when emiting MisExpect diagnostics
+  Optional DiagnosticsMisExpectTolerance = 0;

s/tollerate/tolerate/



Comment at: llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll:3
+
+; RUN: opt < %s -lower-expect -pgo-instr-use 
-pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 
-pass-remarks=misexpect 2>&1 | FileCheck %s
+

tejohnson wrote:
> The old PM is gone in the optimization pipeline, so these old PM lines in 
> this and other tests and the comment about the new PM can go away.
I see you removed the NewPM-style invocations and kept the legacy ones - afaict 
opt will now convert the legacy style pass invocations to the new PM, but I 
think 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth marked 10 inline comments as done.
paulkirth added inline comments.



Comment at: clang/test/Profile/misexpect-branch.c:25
+  int x = 0;
+  if (likely(rando % (outer_loop * inner_loop) == 0)) { // exact-warning-re 
{{Potential performance regression from use of __builtin_expect(): Annotation 
was correct on {{.+}}% ({{[0-9]+ / [0-9]+}}) of profiled executions.}}
+x = baz(rando);

tejohnson wrote:
> Can you add a case where an unlikely() is wrong?
I've added one, but I'm not sure it tests anything meaningfully, since an 
unlikely path being too hot is really the same check as a likely path being too 
cold. 



Comment at: llvm/docs/MisExpect.rst:2
+===
+Misexpect
+===

tejohnson wrote:
> paulkirth wrote:
> > tejohnson wrote:
> > > I don't see a mention of -Wmisexpect in the document. Should there be 
> > > user-facing clang documentation, this seems more specific to the LLVM 
> > > implementation?
> > I will update the patch to include similar documentation this for clang.
> > 
> > Additionally, my understanding is that documentation for warnings is 
> > auto-generated from the tablegen, so that at least will be available 
> > automatically.
> > 
> > 
> Should the clang documentation already be added to this patch? I couldn't 
> find it.
In the last update the clang documentation was a bit too close to the llvm 
documentation. I think they are distinct enough now to be separately useful, 
but maybe it's better to merge the two?



Comment at: llvm/docs/MisExpect.rst:54
+
+.. option:: -pgo-warn-misexpect
+

tejohnson wrote:
> Looking at the code, the -pgo-warn-misexpect seems to be useful for internal 
> LLVM testing via 'opt', to mimic -Wmisexpect, so it probably should be 
> documented as such.
Yes, its useful to test llvm, but shouldn't `opt` be usable in the same way? To 
me it seems useful for opt to support the warning directly too, but I'm happy 
to defer here if you think that's confusing or shouldn't be the case.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:78
+
+Instruction *getOprndOrInst(Instruction *I) {
+  assert(I != nullptr && "MisExpect target Instruction cannot be nullptr");

tejohnson wrote:
> Suggest renaming to something more intuitive. E.g. getInstCondition? 
That's definitely a better name. Thanks for the suggestion.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:188
+  auto Tollerance = getMisExpectTollerance(I.getContext());
+  if (Tollerance < 0)
+Tollerance = 0;

tejohnson wrote:
> afaict this handling duplicates what is done in parseTolleranceOption when 
> the tolerance comes in from the clang option. Can you just do this handling 
> once, here?
This is now the only place we check/modify the value.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 415990.
paulkirth added a comment.

Address comments.

Added clang documentation.

Fixed various typos.

Updated tests per comments for clang and LLVM.

Removed dead class definition.

Removed clamping from tolerance parser.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/docs/MisExpect.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/include/llvm/Transforms/Utils/MisExpectToleranceParser.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-threshold.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,286 @@
+; Test misexpect diagnostics handle swich statements, and report source locations correctly
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:30: Potential performance 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:425
+  /// values in order to be included in misexpect diagnostics.
+  Optional DiagnosticsMisExpectTollerance = 0;
+

s/Tollerance/Tolerance/ (and elsewhere throughout patch)



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1967
+Diags.Report(diag::warn_drv_diagnostics_misexpect_requires_pgo)
+<< "-fdiagnostics-misexpect-tollerance=";
+}

Add tests for expected errors?



Comment at: clang/test/Profile/misexpect-branch.c:25
+  int x = 0;
+  if (likely(rando % (outer_loop * inner_loop) == 0)) { // exact-warning-re 
{{Potential performance regression from use of __builtin_expect(): Annotation 
was correct on {{.+}}% ({{[0-9]+ / [0-9]+}}) of profiled executions.}}
+x = baz(rando);

Can you add a case where an unlikely() is wrong?



Comment at: llvm/docs/MisExpect.rst:2
+===
+Misexpect
+===

paulkirth wrote:
> tejohnson wrote:
> > I don't see a mention of -Wmisexpect in the document. Should there be 
> > user-facing clang documentation, this seems more specific to the LLVM 
> > implementation?
> I will update the patch to include similar documentation this for clang.
> 
> Additionally, my understanding is that documentation for warnings is 
> auto-generated from the tablegen, so that at least will be available 
> automatically.
> 
> 
Should the clang documentation already be added to this patch? I couldn't find 
it.



Comment at: llvm/docs/MisExpect.rst:22
+The MisExpect checks in the LLVM backend follow a simple procedure: if the
+profiling counter associated with an instruction using the ``llvm.expect``
+intrinsic was too low along the expected path, then it emits a diagnostic

paulkirth wrote:
> tejohnson wrote:
> > Suggest using "profile weight" not profiling counter since we don't really 
> > have profile counters associated with instructions (rather with edges in 
> > the MST, at least in the IR PGO).
> > 
> > Also, doesn't the profile weight being too low only handle the LIKELY case? 
> > What about something being marked UNLIKELY with a hot/high profile weight? 
> > edit: After looking through the implementation, I think the comparison only 
> > being done in this direction is based on the way it is implemented, but it 
> > is unclear from this documentation here how the comparison is handling 
> > things being off in either direction.
> Maybe my mental model is off here, but doesn't the `llvm.expect` intrinsic 
> mark a specific value as the 'likely' or expected value? So if you want to 
> mark a branch as unlikely, you're essentially marking the other half of it as 
> 'hot'.
> 
> We could change the comparison to compare all parts of the branch weights 
> too, but the case of an 'unlikely' branch being too hot in my model is 
> captured by the 'likely' branch becoming too 'cold'.
> 
> If that is incorrect, I'd really appreciate some guidance on how to model 
> this more accurately.
Yes I agree that is how llvm.expect is implemented, and I can see from your 
implementation that it should be handling that correctly. It was just unclear 
from the way it is written here what was happening. I like the new wording 
which is less specific to the way the checking is implemented.



Comment at: llvm/docs/MisExpect.rst:39
+``-unlikely-branch-weight`` LLVM options. During verification, if the
+profile count is less than the calculated threshold, then we will emit a
+remark or warning detailing a potential performance regression. The

paulkirth wrote:
> tejohnson wrote:
> > s/count/weight/
> > 
> > Also, similar to my earlier comment, what about an expect UNLIKELY branch 
> > with a high profile weight?
> I think I've reworded this to clarify things a bit more, but let me know if 
> it still needs some polish.
I think the new version is good, thanks.



Comment at: llvm/include/llvm/Transforms/Utils/MisExpectTolleranceParser.h:12
+/// This file implements a simple parser to decode commandline option for
+/// misexpect tollerance that supports both int.
+///

Is the end of the sentence incomplete? I.e. should be "both int and (something 
else)"?



Comment at: llvm/include/llvm/Transforms/Utils/MisExpectTolleranceParser.h:26
+// Parse misexpect tollerance argument value.
+// Valid option values are
+// integer: manually specified threshold

Fix line wrapping. But sentence could be simplified too - is this just saying 
that the input must be an integer?



Comment at: llvm/include/llvm/Transforms/Utils/MisExpectTolleranceParser.h:42
+// A simple CL parser for '-misexpect-tollerance='
+class HotnessThresholdParser : public cl::parser> {
+public:

Where is this class used?

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

MisExpect was originally intended to be quite strict, so that developers would 
audit their code and re-evaluate the correctness of their annotations,or if 
they were needed at all.

I think I'm still of a mind that getting flagged by MisExpect indicates that a 
different annotation would be more beneficial, such as 
`llvm.expect.with.probability`.

Regardless, I agree its good to give users an option to relax the checking when 
they want to, so I will add an option that allows them to specify a scaling 
factor for the threshold.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Please also consider https://github.com/llvm/llvm-project/issues/46534


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth marked 6 inline comments as done.
paulkirth added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:1203
 
+  if (CodeGenOpts.MisExpect) {
+Ctx.setMisExpectWarningRequested(true);

tejohnson wrote:
> Out of curiosity, since I am less familiar with clang than llvm, when is this 
> path taken vs where this is done at line 336 in HandleTranslationUnit?
> 
> Also, it might be nice to have this code placed either consistently before or 
> after the OptRecordFile handling in these two locations, so it is easier to 
> compare.
That was a good catch, I had forgotten to remove this after patching in line 
366. I moved that a bit a few lines lower to fit more nicely, and removed this 
one entirely.



Comment at: llvm/docs/MisExpect.rst:2
+===
+Misexpect
+===

tejohnson wrote:
> I don't see a mention of -Wmisexpect in the document. Should there be 
> user-facing clang documentation, this seems more specific to the LLVM 
> implementation?
I will update the patch to include similar documentation this for clang.

Additionally, my understanding is that documentation for warnings is 
auto-generated from the tablegen, so that at least will be available 
automatically.





Comment at: llvm/docs/MisExpect.rst:22
+The MisExpect checks in the LLVM backend follow a simple procedure: if the
+profiling counter associated with an instruction using the ``llvm.expect``
+intrinsic was too low along the expected path, then it emits a diagnostic

tejohnson wrote:
> Suggest using "profile weight" not profiling counter since we don't really 
> have profile counters associated with instructions (rather with edges in the 
> MST, at least in the IR PGO).
> 
> Also, doesn't the profile weight being too low only handle the LIKELY case? 
> What about something being marked UNLIKELY with a hot/high profile weight? 
> edit: After looking through the implementation, I think the comparison only 
> being done in this direction is based on the way it is implemented, but it is 
> unclear from this documentation here how the comparison is handling things 
> being off in either direction.
Maybe my mental model is off here, but doesn't the `llvm.expect` intrinsic mark 
a specific value as the 'likely' or expected value? So if you want to mark a 
branch as unlikely, you're essentially marking the other half of it as 'hot'.

We could change the comparison to compare all parts of the branch weights too, 
but the case of an 'unlikely' branch being too hot in my model is captured by 
the 'likely' branch becoming too 'cold'.

If that is incorrect, I'd really appreciate some guidance on how to model this 
more accurately.



Comment at: llvm/docs/MisExpect.rst:39
+``-unlikely-branch-weight`` LLVM options. During verification, if the
+profile count is less than the calculated threshold, then we will emit a
+remark or warning detailing a potential performance regression. The

tejohnson wrote:
> s/count/weight/
> 
> Also, similar to my earlier comment, what about an expect UNLIKELY branch 
> with a high profile weight?
I think I've reworded this to clarify things a bit more, but let me know if it 
still needs some polish.



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:109
 
+  SI.setCondition(ArgValue);
+

tejohnson wrote:
> Is there a reason why this line has changed ordering w.r.t. setMetadata?
I think I actually restored this to what it was before the original MisExpect 
patch, but there is no need for that, so I've removed it.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:170
+
+  if (ProfileCount < ScaledThreshold)
+emitMisexpectDiagnostic(I, Ctx, ProfileCount, CaseTotal);

tejohnson wrote:
> Is this too strict - i.e. what if they are off by only a small amount?
Maybe. I don't know what a good default fudge factor would be, though. Maybe 
use 5%, and expose a flag that can override it?

Another approach is that the values used by `llvm.expect` can be changed via 
clang flags, which will update the weights checked against. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 414230.
paulkirth added a comment.

Consolodate common code, clarify documentation, and address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,290 @@
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.
Herald added a project: All.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:172
 CODEGENOPT(NoWarn, 1, 0) ///< Set when -Wa,--no-warn is enabled.
+CODEGENOPT(MisExpect , 1, 0) ///< Set -Wmisexpect is enabled
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.

"Set when ..."



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:1203
 
+  if (CodeGenOpts.MisExpect) {
+Ctx.setMisExpectWarningRequested(true);

Out of curiosity, since I am less familiar with clang than llvm, when is this 
path taken vs where this is done at line 336 in HandleTranslationUnit?

Also, it might be nice to have this code placed either consistently before or 
after the OptRecordFile handling in these two locations, so it is easier to 
compare.



Comment at: llvm/docs/MisExpect.rst:2
+===
+Misexpect
+===

I don't see a mention of -Wmisexpect in the document. Should there be 
user-facing clang documentation, this seems more specific to the LLVM 
implementation?



Comment at: llvm/docs/MisExpect.rst:22
+The MisExpect checks in the LLVM backend follow a simple procedure: if the
+profiling counter associated with an instruction using the ``llvm.expect``
+intrinsic was too low along the expected path, then it emits a diagnostic

Suggest using "profile weight" not profiling counter since we don't really have 
profile counters associated with instructions (rather with edges in the MST, at 
least in the IR PGO).

Also, doesn't the profile weight being too low only handle the LIKELY case? 
What about something being marked UNLIKELY with a hot/high profile weight? 
edit: After looking through the implementation, I think the comparison only 
being done in this direction is based on the way it is implemented, but it is 
unclear from this documentation here how the comparison is handling things 
being off in either direction.



Comment at: llvm/docs/MisExpect.rst:27
+The most natural place to perform the verification is just prior to when
+branch weights being assigned to the target instruction in the form of
+branch weight metadata.

grammatical issue "prior to when ... being assigned" - suggest "are assigned".



Comment at: llvm/docs/MisExpect.rst:39
+``-unlikely-branch-weight`` LLVM options. During verification, if the
+profile count is less than the calculated threshold, then we will emit a
+remark or warning detailing a potential performance regression. The

s/count/weight/

Also, similar to my earlier comment, what about an expect UNLIKELY branch with 
a high profile weight?



Comment at: llvm/docs/MisExpect.rst:48
+
+.. option:: -pass-remarks=misexpect
+

As noted in my comment at the top, it would be good to have user-facing clang 
documentation, in which case the clang version of this option is 
-Rpass=misexpect. 



Comment at: llvm/docs/MisExpect.rst:54
+
+.. option:: -pgo-warn-misexpect
+

Looking at the code, the -pgo-warn-misexpect seems to be useful for internal 
LLVM testing via 'opt', to mimic -Wmisexpect, so it probably should be 
documented as such.



Comment at: llvm/include/llvm/Transforms/Utils/MisExpect.h:39
+
+/// checkBackendInstrumentation - compares PGO counters to the thresholds used
+/// for llvm.expect and warns if the PGO counters are outside of the expected

Update function name.



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:109
 
+  SI.setCondition(ArgValue);
+

Is there a reason why this line has changed ordering w.r.t. setMetadata?



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:157
+  const uint64_t UnlikelyBranchWeight = ExpectedWeights[MinId];
+  const uint64_t ProfileCount = RealWeights[Index];
+  const uint64_t CaseTotal =

This is the only use of Index - can you just use MaxId directly? I guess the 
assumption is that the ExpectedWeights array has the same ordering as the 
RealWeights array, so we can check if the corresponding real weight from 
profile is colder than the most likely weight from the expect. Some comments to 
describe how the comparison is being done in this code more intuitively would 
help make it easier to understand.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:166
+
+  const llvm::BranchProbability LikelyThreshold(LikelyBranchWeight,
+TotalBranchWeight);

Why is this called a "Threshold" and not just a "Probability"?



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:170
+
+  if (ProfileCount < ScaledThreshold)
+emitMisexpectDiagnostic(I, Ctx, ProfileCount, 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 395727.
paulkirth added a comment.

- [misexpect] Add missing tests and modify diagnostic


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,290 @@
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 395517.
paulkirth added a comment.

- [misexpect] Add missing tests and modify diagnostic


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,290 @@
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 395502.
paulkirth added a comment.

- [misexpect] Add missing tests and modify diagnostic


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115907/new/

https://reviews.llvm.org/D115907

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,290 @@
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:30: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:30: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:30: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of 

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

[Paul Kirth via Phabricator via cfe-commits]

paulkirth created this revision.
Herald added subscribers: abrachet, ormris, dexonsmith, wenlei, phosek, 
hiraditya, mgorny.
paulkirth updated this revision to Diff 394995.
paulkirth added a comment.
paulkirth updated this revision to Diff 394997.
paulkirth added reviewers: phosek, mcgrathr, leonardchan, lebedev.ri.
paulkirth published this revision for review.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

[misexpect] Fix Typo


paulkirth added a comment.

Fix bad diff in arctool


paulkirth added a comment.

Ready for review


Reimplements MisExpect diagnostics to reconstruct its original checking
methodology only using MD_prof branch_weights metadata.

New checks rely on 2 invariants:

1. for frontend instrumentation, MD_prof branch_weights will always be 
populated before llvm.expect intrinsics are lowered.

2. for IR and sample profiling, llvm.expect intrinsics will always be lowered 
before branch_weights are populated from the IR profiles.

These invariants allow the checking to assume how the existing branch
weights are populated depending on the profiling method used, and emit
the correct diagnostics. If these invariants are ever invalidated, the
MisExpect related checks would need to be updated, potentially by
re-introducing MD_misexpect metadata, and ensuring it always will be
transformed the same way as branch_weights in other optimization passes.

Frontend based profiling is now enabled without using LLVM Args, by
introducing a new CodeGen option, and checking if the -Wmisexpect flag
has been passed on the command line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115907

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,290 @@
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt <