[PATCH] D134394: Update docs about [[likely]] vs. PGO

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

See also D134456  which affects the behavior 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134394

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

[PATCH] D134394: Update docs about [[likely]] vs. PGO

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134394#3808397 , 
@serge-sans-paille wrote:

> While running this small experiment: 
> https://sergesanspaille.fedorapeople.org/proftest.sh

Thank you for the help with testing the assumptions here!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134394

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

[PATCH] D134394: Update docs about [[likely]] vs. PGO

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

While running this small experiment: 
https://sergesanspaille.fedorapeople.org/proftest.sh
I get the following output:

  clang version 16.0.0 (...)
  Target: x86_64-unknown-linux-gnu
  Thread model: posix
  InstalledDir: ...
   - instrumentation + likely: puts first
  
  06d0 :
   6d0: 50  push   %rax
   6d1: 83 ff 01cmp$0x1,%edi
   6d4: 75 10   jne6e6 
   6d6: 48 8d 3d a7 00 00 00lea0xa7(%rip),%rdi# 784 
<_IO_stdin_used+0x4>
   6dd: e8 ce fe ff ff  callq  5b0 
   6e2: 31 c0   xor%eax,%eax
   6e4: 59  pop%rcx
   6e5: c3  retq   
   6e6: 48 8b 06mov(%rsi),%rax
   6e9: 48 8b 56 08 mov0x8(%rsi),%rdx
   6ed: 48 8d 3d 9b 00 00 00lea0x9b(%rip),%rdi# 78f 
<_IO_stdin_used+0xf>
   6f4: 48 89 c6mov%rax,%rsi
   6f7: 31 c0   xor%eax,%eax
   6f9: e8 92 fe ff ff  callq  590 
   6fe: eb e2   jmp6e2 
  
  0700 <__libc_csu_init>:
  
    + instrumentation + likely: puts first
  
  05e0 :
   5e0: 50  push   %rax
   5e1: 83 ff 01cmp$0x1,%edi
   5e4: 75 10   jne5f6 
   5e6: 48 8d 3d 97 01 00 00lea0x197(%rip),%rdi# 784 
<_IO_stdin_used+0x4>
   5ed: e8 be ff ff ff  callq  5b0 
   5f2: 31 c0   xor%eax,%eax
   5f4: 59  pop%rcx
   5f5: c3  retq   
   5f6: 48 8b 06mov(%rsi),%rax
   5f9: 48 8b 56 08 mov0x8(%rsi),%rdx
   5fd: 48 8d 3d 8b 01 00 00lea0x18b(%rip),%rdi# 78f 
<_IO_stdin_used+0xf>
   604: 48 89 c6mov%rax,%rsi
   607: 31 c0   xor%eax,%eax
   609: e8 82 ff ff ff  callq  590 
   60e: eb e2   jmp5f2 
  
  0610 <_start>:
  ./b1.out 12
   - instrumentation - likely: puts first
  
  06d0 :
   6d0: 50  push   %rax
   6d1: 83 ff 01cmp$0x1,%edi
   6d4: 75 0e   jne6e4 
   6d6: 48 8d 3d a7 00 00 00lea0xa7(%rip),%rdi# 784 
<_IO_stdin_used+0x4>
   6dd: e8 ce fe ff ff  callq  5b0 
   6e2: eb 18   jmp6fc 
   6e4: 48 8b 06mov(%rsi),%rax
   6e7: 48 8b 56 08 mov0x8(%rsi),%rdx
   6eb: 48 8d 3d 9d 00 00 00lea0x9d(%rip),%rdi# 78f 
<_IO_stdin_used+0xf>
   6f2: 48 89 c6mov%rax,%rsi
   6f5: 31 c0   xor%eax,%eax
   6f7: e8 94 fe ff ff  callq  590 
   6fc: 31 c0   xor%eax,%eax
   6fe: 59  pop%rcx
   6ff: c3  retq   
  
  0700 <__libc_csu_init>:
  
    + instrumentation - likely: printf first
  
  05e0 :
   5e0: 50  push   %rax
   5e1: 83 ff 01cmp$0x1,%edi
   5e4: 74 1c   je 602 
   5e6: 48 8b 06mov(%rsi),%rax
   5e9: 48 8b 56 08 mov0x8(%rsi),%rdx
   5ed: 48 8d 3d 9b 01 00 00lea0x19b(%rip),%rdi# 78f 
<_IO_stdin_used+0xf>
   5f4: 48 89 c6mov%rax,%rsi
   5f7: 31 c0   xor%eax,%eax
   5f9: e8 92 ff ff ff  callq  590 
   5fe: 31 c0   xor%eax,%eax
   600: 59  pop%rcx
   601: c3  retq   
   602: 48 8d 3d 7b 01 00 00lea0x17b(%rip),%rdi# 784 
<_IO_stdin_used+0x4>
   609: e8 a2 ff ff ff  callq  5b0 
   60e: eb ee   jmp5fe 
  
  0610 <_start>:

To me, it means that `[[likely]]` takes precedence over profile information...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134394

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

[PATCH] D134394: Update docs about [[likely]] vs. PGO

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.
hans added reviewers: Mordante, aaron.ballman.
Herald added a subscriber: wenlei.
Herald added a project: All.
hans requested review of this revision.
Herald added a project: clang.

It sounds like these attributes do nothing when PGO is enabled, which is 
actually (or please correct me if I'm wrong) only the case when there is 
profile data coverage the specific statements in question. If there is no 
profile data, the attributes still apply.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134394

Files:
  clang/include/clang/Basic/AttrDocs.td


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1963,8 +1963,9 @@
 likelihood. The positive likelihood is the more likely of path of execution
 than the neutral likelihood.
 
-These attributes have no effect on the generated code when using
-PGO (Profile-Guided Optimization) or at optimization level 0.
+These attributes have no effect on the generated code when there is PGO
+(Profile-Guided Optimization) data available for the statements, or at
+optimization level 0.
 
 In Clang, the attributes will be ignored if they're not placed on
 * the ``case`` or ``default`` label of a ``switch`` statement,


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1963,8 +1963,9 @@
 likelihood. The positive likelihood is the more likely of path of execution
 than the neutral likelihood.
 
-These attributes have no effect on the generated code when using
-PGO (Profile-Guided Optimization) or at optimization level 0.
+These attributes have no effect on the generated code when there is PGO
+(Profile-Guided Optimization) data available for the statements, or at
+optimization level 0.
 
 In Clang, the attributes will be ignored if they're not placed on
 * the ``case`` or ``default`` label of a ``switch`` statement,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits