Re: [PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hans Wennborg via cfe-commits]

On Wed, Aug 5, 2020 at 6:26 PM Hiroshi Yamauchi via llvm-commits
 wrote:
>
>
>
> On Wed, Aug 5, 2020 at 8:30 AM Hans Wennborg via Phabricator 
>  wrote:
>>
>> hans added a comment.
>>
>> In D84782#2191429 , @yamauchi wrote:
>>
>> > In D84782#2191243 , @MaskRay 
>> > wrote:
>> >
>> >> @yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 
>> >>  and 
>> >> 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d 
>> >>  in 
>> >> https://github.com/llvm/llvm-project/tree/release/11.x ?
>> >
>> > I think it'd be good to have them, if possible, though it's a latent, 
>> > non-recent bug.
>>
>> They're hard to apply without 50da55a58534e9207d8d5e31c8b4b5cf0c624175 
>> . Do 
>> you think we should take that also? Or maybe this can wait since it's not a 
>> new bug.
>
>
> 50da55a58534e9207d8d5e31c8b4b5cf0c624175 isn't a bug fix but it's not 
> changing the default behavior. So, it is probably safe to take it. That said, 
> I'd say this can wait unless there's some immediate issue, since it's not a 
> new bug.

Okay, let's not worry about it for 11.x then.

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

Re: [PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via cfe-commits]

On Wed, Aug 5, 2020 at 8:30 AM Hans Wennborg via Phabricator <
revi...@reviews.llvm.org> wrote:

> hans added a comment.
>
> In D84782#2191429 , @yamauchi
> wrote:
>
> > In D84782#2191243 , @MaskRay
> wrote:
> >
> >> @yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 <
> https://reviews.llvm.org/rGcd890944ad344b1b8cac58332ab11c9eec6b61e9> and
> 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d <
> https://reviews.llvm.org/rG3d6f53018f845e893ad34f64ff2851a2e5c3ba1d> in
> https://github.com/llvm/llvm-project/tree/release/11.x ?
> >
> > I think it'd be good to have them, if possible, though it's a latent,
> non-recent bug.
>
> They're hard to apply without 50da55a58534e9207d8d5e31c8b4b5cf0c624175 <
> https://reviews.llvm.org/rG50da55a58534e9207d8d5e31c8b4b5cf0c624175>. Do
> you think we should take that also? Or maybe this can wait since it's not a
> new bug.
>

50da55a58534e9207d8d5e31c8b4b5cf0c624175 isn't a bug fix but it's not
changing the default behavior. So, it is probably safe to take it. That
said, I'd say this can wait unless there's some immediate issue, since it's
not a new bug.


>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D84782/new/
>
> https://reviews.llvm.org/D84782
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D84782#2191429 , @yamauchi wrote:

> In D84782#2191243 , @MaskRay wrote:
>
>> @yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 
>>  and 
>> 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d 
>>  in 
>> https://github.com/llvm/llvm-project/tree/release/11.x ?
>
> I think it'd be good to have them, if possible, though it's a latent, 
> non-recent bug.

They're hard to apply without 50da55a58534e9207d8d5e31c8b4b5cf0c624175 
. Do you 
think we should take that also? Or maybe this can wait since it's not a new bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

In D84782#2191243 , @MaskRay wrote:

> @yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 
>  and 
> 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d 
>  in 
> https://github.com/llvm/llvm-project/tree/release/11.x ?

I think it'd be good to have them, if possible, though it's a latent, 
non-recent bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a subscriber: hans.
MaskRay added a comment.

@yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 
 and 
3d6f53018f845e893ad34f64ff2851a2e5c3ba1d 
 in 
https://github.com/llvm/llvm-project/tree/release/11.x ?

+@hans


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

Yup, -pgo-instr-old-cfg-hashing=true to revert back to the old hashing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

It is also a convention to explain what has changed compared with the initial 
land.

As a hindsight, the option `-pgo-instr-old-cfg-hashing` should probably have 
been mentioned in the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay closed this revision.
MaskRay added a comment.

Relanded as 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d 


@yamauchi For a reland, you still need to include `Differential Revision: ` so 
that the differential can be closed automatically when you push the commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

Latest update diff:

  < +  std::vector Data;
  < +  for (unsigned I = 0; I < 8; ++I)
  < +Data.push_back((uint8_t)(Num >> (I * 8)));
  ---
  > +  uint8_t Data[8];
  > +  support::endian::write64le(Data, Num);




Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:661
+} Data;
+Data.N = (uint64_t)SIVisitor.getNumOfSelectInsts();
+JCH.update(Data.C);

yamauchi wrote:
> MaskRay wrote:
> > MaskRay wrote:
> > > `#include "llvm/Support/Endian.h"`
> > > 
> > > and use `support::endian::write64le(, 
> > > (uint64_t)SIVisitor.getNumOfSelectInsts())`
> > Sorry, just use a plain `uint8_t C[8];` and `support::endian::write64le(C, 
> > (uint64_t)SIVisitor.getNumOfSelectInsts())`
> I went for the same style (using a vector) as the above code (line 642).
Now uses write64le.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281764.
yamauchi added a comment.

Use endian::write64le.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll

Index: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
===
--- llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
+++ llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
@@ -1,6 +1,8 @@
 ; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata
 ; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata  -S | FileCheck %s
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -29,6 +31,9 @@
 ; CHECK: %mul.i = select i1 %cmp.i, i32 1, i32 %i
 ; CHECK-SAME: !prof ![[BW:[0-9]+]]
 ; CHECK: ![[BW]] = !{!"branch_weights", i32 12, i32 6}
+; CHECKOLDHASH: %mul.i = select i1 %cmp.i, i32 1, i32 %i
+; CHECKOLDHASH-SAME: !prof ![[BW:[0-9]+]]
+; CHECKOLDHASH: ![[BW]] = !{!"branch_weights", i32 6, i32 12}
   %retval.0.i = mul nsw i32 %mul.i, %i
   ret i32 %retval.0.i
 }
Index: llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
===
--- llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
+++ 

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

Here's the diff in the latest update:

  < +union {
  < +  uint64_t N;
  < +  uint8_t C[8];
  < +} Data;
  < +Data.N = (uint64_t)SIVisitor.getNumOfSelectInsts();
  < +JCH.update(Data.C);
  < +Data.N = (uint64_t)ValueSites[IPVK_IndirectCallTarget].size();
  < +JCH.update(Data.C);
  < +Data.N = (uint64_t)ValueSites[IPVK_MemOPSize].size();
  < +JCH.update(Data.C);
  < +Data.N = (uint64_t)MST.AllEdges.size();
  < +JCH.update(Data.C);
  ---
  > +auto updateJCH = [](uint64_t Num) {
  > +  std::vector Data;
  > +  for (unsigned I = 0; I < 8; ++I)
  > +Data.push_back((uint8_t)(Num >> (I * 8)));
  > +  JCH.update(Data);
  > +};
  > +updateJCH((uint64_t)SIVisitor.getNumOfSelectInsts());
  > +updateJCH((uint64_t)ValueSites[IPVK_IndirectCallTarget].size());
  > +updateJCH((uint64_t)ValueSites[IPVK_MemOPSize].size());
  > +updateJCH((uint64_t)MST.AllEdges.size());


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:661
+} Data;
+Data.N = (uint64_t)SIVisitor.getNumOfSelectInsts();
+JCH.update(Data.C);

MaskRay wrote:
> MaskRay wrote:
> > `#include "llvm/Support/Endian.h"`
> > 
> > and use `support::endian::write64le(, 
> > (uint64_t)SIVisitor.getNumOfSelectInsts())`
> Sorry, just use a plain `uint8_t C[8];` and `support::endian::write64le(C, 
> (uint64_t)SIVisitor.getNumOfSelectInsts())`
I went for the same style (using a vector) as the above code (line 642).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281760.
yamauchi added a comment.

Fix the endianness issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll

Index: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
===
--- llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
+++ llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
@@ -1,6 +1,8 @@
 ; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata
 ; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata  -S | FileCheck %s
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -29,6 +31,9 @@
 ; CHECK: %mul.i = select i1 %cmp.i, i32 1, i32 %i
 ; CHECK-SAME: !prof ![[BW:[0-9]+]]
 ; CHECK: ![[BW]] = !{!"branch_weights", i32 12, i32 6}
+; CHECKOLDHASH: %mul.i = select i1 %cmp.i, i32 1, i32 %i
+; CHECKOLDHASH-SAME: !prof ![[BW:[0-9]+]]
+; CHECKOLDHASH: ![[BW]] = !{!"branch_weights", i32 6, i32 12}
   %retval.0.i = mul nsw i32 %mul.i, %i
   ret i32 %retval.0.i
 }
Index: llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
===
--- llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
+++ 

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:661
+} Data;
+Data.N = (uint64_t)SIVisitor.getNumOfSelectInsts();
+JCH.update(Data.C);

MaskRay wrote:
> `#include "llvm/Support/Endian.h"`
> 
> and use `support::endian::write64le(, 
> (uint64_t)SIVisitor.getNumOfSelectInsts())`
Sorry, just use a plain `uint8_t C[8];` and `support::endian::write64le(C, 
(uint64_t)SIVisitor.getNumOfSelectInsts())`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:661
+} Data;
+Data.N = (uint64_t)SIVisitor.getNumOfSelectInsts();
+JCH.update(Data.C);

`#include "llvm/Support/Endian.h"`

and use `support::endian::write64le(, 
(uint64_t)SIVisitor.getNumOfSelectInsts())`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

Re: [PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Xinliang David Li via cfe-commits]

right. It occurred to me during review, but did not think of the hard coded
values in proftext depends on LE.

On Wed, Jul 29, 2020 at 3:04 PM Hiroshi Yamauchi via Phabricator <
revi...@reviews.llvm.org> wrote:

> yamauchi added a comment.
>
> Builtbot failure:
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/51984
>
> I think it's an endian issue in the hash computation.
>
> Will revert.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D84782/new/
>
> https://reviews.llvm.org/D84782
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

Builtbot failure: 
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/51984

I think it's an endian issue in the hash computation.

Will revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi 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 rG120e66b3418b: [PGO] Include the mem ops into the function 
hash. (authored by yamauchi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll

Index: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
===
--- llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
+++ llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
@@ -1,6 +1,8 @@
 ; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata
 ; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata  -S | FileCheck %s
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -29,6 +31,9 @@
 ; CHECK: %mul.i = select i1 %cmp.i, i32 1, i32 %i
 ; CHECK-SAME: !prof ![[BW:[0-9]+]]
 ; CHECK: ![[BW]] = !{!"branch_weights", i32 12, i32 6}
+; CHECKOLDHASH: %mul.i = select i1 %cmp.i, i32 1, i32 %i
+; CHECKOLDHASH-SAME: !prof ![[BW:[0-9]+]]
+; CHECKOLDHASH: ![[BW]] = !{!"branch_weights", i32 6, i32 12}
   %retval.0.i = mul nsw i32 %mul.i, %i
   ret i32 %retval.0.i
 }
Index: llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[David Li via Phabricator via cfe-commits]

davidxl accepted this revision.
davidxl added a comment.

lgtm

Just realized that we need a test case to show it fixes the original issue 
(existence with memop --> different hash). Ok as a follow up .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281716.
yamauchi added a comment.

Address comments. PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll

Index: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
===
--- llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
+++ llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
@@ -1,6 +1,8 @@
 ; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata
 ; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata  -S | FileCheck %s
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck -check-prefix=CHECKOLDHASH %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
@@ -29,6 +31,9 @@
 ; CHECK: %mul.i = select i1 %cmp.i, i32 1, i32 %i
 ; CHECK-SAME: !prof ![[BW:[0-9]+]]
 ; CHECK: ![[BW]] = !{!"branch_weights", i32 12, i32 6}
+; CHECKOLDHASH: %mul.i = select i1 %cmp.i, i32 1, i32 %i
+; CHECKOLDHASH-SAME: !prof ![[BW:[0-9]+]]
+; CHECKOLDHASH: ![[BW]] = !{!"branch_weights", i32 6, i32 12}
   %retval.0.i = mul nsw i32 %mul.i, %i
   ret i32 %retval.0.i
 }
Index: llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
===
--- llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
+++ 

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added inline comments.



Comment at: 
llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext:20
 18
 12
 

davidxl wrote:
> nit: change 12 to a different value say 6.(otherwise if the option is an nop, 
> the test will still succeed).
Done.



Comment at: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll:3
 ; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata  -S | 
FileCheck %s
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata 
-pgo-instr-old-cfg-hashing=true -S | FileCheck %s
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | 
FileCheck %s

davidxl wrote:
> Use a different prefix like CHECKOLD
Done.



Comment at: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll:33
 ; CHECK-SAME: !prof ![[BW:[0-9]+]]
 ; CHECK: ![[BW]] = !{!"branch_weights", i32 12, i32 6}
   %retval.0.i = mul nsw i32 %mul.i, %i

davidxl wrote:
> CHECKOLD expects a different weight.
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

LGTM as well with @davidxl's comments addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[David Li via Phabricator via cfe-commits]

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

lgtm (with the small test enhancement)




Comment at: 
llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext:20
 18
 12
 

nit: change 12 to a different value say 6.(otherwise if the option is an nop, 
the test will still succeed).



Comment at: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll:3
 ; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata  -S | 
FileCheck %s
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata 
-pgo-instr-old-cfg-hashing=true -S | FileCheck %s
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | 
FileCheck %s

Use a different prefix like CHECKOLD



Comment at: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll:33
 ; CHECK-SAME: !prof ![[BW:[0-9]+]]
 ; CHECK: ![[BW]] = !{!"branch_weights", i32 12, i32 6}
   %retval.0.i = mul nsw i32 %mul.i, %i

CHECKOLD expects a different weight.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added inline comments.



Comment at: 
llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext:1
 # IR level Instrumentation Flag
 :ir

davidxl wrote:
> We can convert this test case to cover the new option introduced:
> 
> basically create another profile entry for _Z3fooi  with the old hash. Change 
> the counter value  a little and expect the profile-use match the old entry 
> when the option is used.
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281708.
yamauchi added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll

Index: llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
===
--- llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
+++ llvm/test/Transforms/PGOProfile/multiple_hash_profile.ll
@@ -1,6 +1,8 @@
 ; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata
 ; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata  -S | FileCheck %s
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck %s
 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-instr-old-cfg-hashing=true -S | FileCheck %s
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
Index: llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
===
--- llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
+++ llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
@@ -10,7 +10,7 @@
 
 foo
 # Func Hash:
-29212902728
+1720106746050921044
 # Num Counters:
 2
 # Counter Values:
@@ -19,7 +19,7 @@
 
 bar
 # Func Hash:
-1152921534274394772
+1299757151682747028
 # Num Counters:
 2
 # Counter Values:
@@ -45,7 +45,7 @@
 
 main
 # Func Hash:
-12884901887
+1895182923573755903
 # Num Counters:
 1
 # Counter Values:
@@ -53,7 +53,7 @@
 
 cspgo.c:foo
 # Func Hash:
-1152921563228422740

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.



Comment at: 
llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext:1
 # IR level Instrumentation Flag
 :ir

We can convert this test case to cover the new option introduced:

basically create another profile entry for _Z3fooi  with the old hash. Change 
the counter value  a little and expect the profile-use match the old entry when 
the option is used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281673.
yamauchi added a comment.

Rebase past D84865 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext

Index: llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
===
--- llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
+++ llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
@@ -10,7 +10,7 @@
 
 foo
 # Func Hash:
-29212902728
+1720106746050921044
 # Num Counters:
 2
 # Counter Values:
@@ -19,7 +19,7 @@
 
 bar
 # Func Hash:
-1152921534274394772
+1299757151682747028
 # Num Counters:
 2
 # Counter Values:
@@ -45,7 +45,7 @@
 
 main
 # Func Hash:
-12884901887
+1895182923573755903
 # Num Counters:
 1
 # Counter Values:
@@ -53,7 +53,7 @@
 
 cspgo.c:foo
 # Func Hash:
-1152921563228422740
+1720106746050921044
 # Num Counters:
 4
 # Counter Values:
Index: llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
===
--- llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
+++ llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
@@ -2,7 +2,7 @@
 :ir
 :entry_first
 test_switch
-46200943743
+536873293052540031
 4
 10
 5
Index: llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
===
--- llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
+++ llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
@@ -1,7 +1,7 @@
 # :ir is the flag to indicate this is IR level profile.
 :ir
 test_switch
-46200943743
+536873293052540031
 4
 0
 5
Index: llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
===

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

In D84782#2180626 , @davidxl wrote:

> changes like in llvm/test/Transforms/PGOProfile/PR41279.ll etc can be 
> independently committed.

Uploaded D84865  for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:266
+PGOOldCFGHashing("pgo-instr-old-cfg-hashing", cl::init(false), cl::Hidden,
+ cl::desc("Use the old CFG function hashing."));
+

MaskRay wrote:
> Nit: descriptions usually don't end with a period
> 
> (I know that some descriptions in this file are not consistent)
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281626.
yamauchi added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/PR41279.ll
  llvm/test/Transforms/PGOProfile/PR41279_2.ll
  llvm/test/Transforms/PGOProfile/branch1.ll
  llvm/test/Transforms/PGOProfile/branch2.ll
  llvm/test/Transforms/PGOProfile/criticaledge.ll
  llvm/test/Transforms/PGOProfile/instr_entry_bb.ll
  llvm/test/Transforms/PGOProfile/landingpad.ll
  llvm/test/Transforms/PGOProfile/loop1.ll
  llvm/test/Transforms/PGOProfile/loop2.ll
  llvm/test/Transforms/PGOProfile/memop_size_from_strlen.ll
  llvm/test/Transforms/PGOProfile/single_bb.ll
  llvm/test/Transforms/PGOProfile/switch.ll

Index: llvm/test/Transforms/PGOProfile/switch.ll
===
--- llvm/test/Transforms/PGOProfile/switch.ll
+++ llvm/test/Transforms/PGOProfile/switch.ll
@@ -19,7 +19,7 @@
 entry:
 ; GEN: entry:
 ; NOTENTRY-NOT: call void @llvm.instrprof.increment
-; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 0)
+; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 {{[0-9]+}}, i32 4, i32 0)
   switch i32 %i, label %sw.default [
 i32 1, label %sw.bb
 i32 2, label %sw.bb1
@@ -31,23 +31,23 @@
 
 sw.bb:
 ; GEN: sw.bb:
-; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 2)
+; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 {{[0-9]+}}, i32 

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:266
+PGOOldCFGHashing("pgo-instr-old-cfg-hashing", cl::init(false), cl::Hidden,
+ cl::desc("Use the old CFG function hashing."));
+

Nit: descriptions usually don't end with a period

(I know that some descriptions in this file are not consistent)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

changes like in llvm/test/Transforms/PGOProfile/PR41279.ll etc can be 
independently committed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

In D84782#2180578 , @davidxl wrote:

> Can you split out the test changes and commit it separately?

How exactly? Do you mean commit the non-test part without flipping the flag and 
then flip it with the test changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281426.
yamauchi added a comment.

Rebase (converting one more test) and fixing a typo and whitespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/suppl-profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/PR41279.ll
  llvm/test/Transforms/PGOProfile/PR41279_2.ll
  llvm/test/Transforms/PGOProfile/branch1.ll
  llvm/test/Transforms/PGOProfile/branch2.ll
  llvm/test/Transforms/PGOProfile/criticaledge.ll
  llvm/test/Transforms/PGOProfile/instr_entry_bb.ll
  llvm/test/Transforms/PGOProfile/landingpad.ll
  llvm/test/Transforms/PGOProfile/loop1.ll
  llvm/test/Transforms/PGOProfile/loop2.ll
  llvm/test/Transforms/PGOProfile/memop_size_from_strlen.ll
  llvm/test/Transforms/PGOProfile/single_bb.ll
  llvm/test/Transforms/PGOProfile/switch.ll

Index: llvm/test/Transforms/PGOProfile/switch.ll
===
--- llvm/test/Transforms/PGOProfile/switch.ll
+++ llvm/test/Transforms/PGOProfile/switch.ll
@@ -19,7 +19,7 @@
 entry:
 ; GEN: entry:
 ; NOTENTRY-NOT: call void @llvm.instrprof.increment
-; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 0)
+; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 {{[0-9]+}}, i32 4, i32 0)
   switch i32 %i, label %sw.default [
 i32 1, label %sw.bb
 i32 2, label %sw.bb1
@@ -31,23 +31,23 @@
 
 sw.bb:
 ; GEN: sw.bb:
-; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 2)
+; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* 

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

Can you split out the test changes and commit it separately?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

In D84782#2180432 , @davidxl wrote:

> On version bump --- looks like FrontPGO has been bumping version for hashing 
> changes while IR PGO had not, so it is ok to leave the version unchanged 
> (current situation is also confusing as the version of IR and FE PGO are 
> mixed).
>
> On the other hand, just in case, we need to introduce a temporary internal 
> option for people to be able to keep on using old profile (with older scheme 
> of hashing) for a while. Basically, under the option, the hashing scheme 
> falls back to the old way.
>
> other than that, looks fine.

Thanks. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281417.
yamauchi added a comment.

Add a flag to back out to the old hashing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/PR41279.ll
  llvm/test/Transforms/PGOProfile/PR41279_2.ll
  llvm/test/Transforms/PGOProfile/branch1.ll
  llvm/test/Transforms/PGOProfile/branch2.ll
  llvm/test/Transforms/PGOProfile/criticaledge.ll
  llvm/test/Transforms/PGOProfile/instr_entry_bb.ll
  llvm/test/Transforms/PGOProfile/landingpad.ll
  llvm/test/Transforms/PGOProfile/loop1.ll
  llvm/test/Transforms/PGOProfile/loop2.ll
  llvm/test/Transforms/PGOProfile/memop_size_from_strlen.ll
  llvm/test/Transforms/PGOProfile/single_bb.ll
  llvm/test/Transforms/PGOProfile/switch.ll

Index: llvm/test/Transforms/PGOProfile/switch.ll
===
--- llvm/test/Transforms/PGOProfile/switch.ll
+++ llvm/test/Transforms/PGOProfile/switch.ll
@@ -19,7 +19,7 @@
 entry:
 ; GEN: entry:
 ; NOTENTRY-NOT: call void @llvm.instrprof.increment
-; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 0)
+; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 {{[0-9]+}}, i32 4, i32 0)
   switch i32 %i, label %sw.default [
 i32 1, label %sw.bb
 i32 2, label %sw.bb1
@@ -31,23 +31,23 @@
 
 sw.bb:
 ; GEN: sw.bb:
-; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 2)
+; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 {{[0-9]+}}, i32 4, i32 2)
   br label %sw.epilog
 
 

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

> Many tests can also be enhanced to filter out the hash details if they are 
> not part the main test.

Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281407.
yamauchi added a comment.

Shift the higher 32 bits by 28 bits and add.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/PR41279.ll
  llvm/test/Transforms/PGOProfile/PR41279_2.ll
  llvm/test/Transforms/PGOProfile/branch1.ll
  llvm/test/Transforms/PGOProfile/branch2.ll
  llvm/test/Transforms/PGOProfile/criticaledge.ll
  llvm/test/Transforms/PGOProfile/instr_entry_bb.ll
  llvm/test/Transforms/PGOProfile/landingpad.ll
  llvm/test/Transforms/PGOProfile/loop1.ll
  llvm/test/Transforms/PGOProfile/loop2.ll
  llvm/test/Transforms/PGOProfile/memop_size_from_strlen.ll
  llvm/test/Transforms/PGOProfile/single_bb.ll
  llvm/test/Transforms/PGOProfile/switch.ll

Index: llvm/test/Transforms/PGOProfile/switch.ll
===
--- llvm/test/Transforms/PGOProfile/switch.ll
+++ llvm/test/Transforms/PGOProfile/switch.ll
@@ -19,7 +19,7 @@
 entry:
 ; GEN: entry:
 ; NOTENTRY-NOT: call void @llvm.instrprof.increment
-; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 0)
+; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 {{[0-9]+}}, i32 4, i32 0)
   switch i32 %i, label %sw.default [
 i32 1, label %sw.bb
 i32 2, label %sw.bb1
@@ -31,23 +31,23 @@
 
 sw.bb:
 ; GEN: sw.bb:
-; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 2)
+; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 {{[0-9]+}}, i32 4, i32 2)
   br label %sw.epilog
 
 

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

On version bump --- looks like FrontPGO has been bumping version for hashing 
changes while IR PGO had not, so it is ok to leave the version unchanged 
(current situation is also confusing as the version of IR and FE PGO are mixed).

On the other hand, just in case, we need to introduce a temporary internal 
option for people to be able to keep on using old profile (with older scheme of 
hashing) for a while. Basically, under the option, the hashing scheme falls 
back to the old way.

other than that, looks fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

We may also need to bump both the raw and index format version with this 
change. For the profile use side, we also need to keep the hashing scheme of 
the older version (in profile-use side).  More details to come.

Many tests can also be enhanced to filter out the hash details if they are not 
part the main test.




Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:623
 // Compute Hash value for the CFG: the lower 32 bits are CRC32 of the index
-// value of each BB in the CFG. The higher 32 bits record the number of edges.
+// value of each BB in the CFG. The higher 32 bits are the CR32 of the numbers
+// of selects, indirect calls, mem ops and edges.

MaskRay wrote:
> To make sure I understand: we use two CRCs instead of one CRC because we want 
> to extract low/high 32 bits for debugging purposes?
> 
> Otherwise, we can just use one CRC.
JamCRC produces a 32bit value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:660
   // information.
-  FunctionHash = (uint64_t)SIVisitor.getNumOfSelectInsts() << 56 |
- (uint64_t)ValueSites[IPVK_IndirectCallTarget].size() << 48 |
- //(uint64_t)ValueSites[IPVK_MemOPSize].size() << 40 |
- (uint64_t)MST.AllEdges.size() << 32 | JC.getCRC();
+  FunctionHash = ((uint64_t)JCH.getCRC()) << 32 | JC.getCRC();
+

Actually I think it'd be better to shift by 28 here rather than throwing out 
the top 4 bits for reservation. Also, adding would be better than or'ing. Will 
update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:623
 // Compute Hash value for the CFG: the lower 32 bits are CRC32 of the index
-// value of each BB in the CFG. The higher 32 bits record the number of edges.
+// value of each BB in the CFG. The higher 32 bits are the CR32 of the numbers
+// of selects, indirect calls, mem ops and edges.

MaskRay wrote:
> To make sure I understand: we use two CRCs instead of one CRC because we want 
> to extract low/high 32 bits for debugging purposes?
> 
> Otherwise, we can just use one CRC.
JamCRC's result is 32-bit, but we have 64-bit (actually 60-bits) of spare room. 
Using more bits should decrease the chance of hash collisions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:623
 // Compute Hash value for the CFG: the lower 32 bits are CRC32 of the index
-// value of each BB in the CFG. The higher 32 bits record the number of edges.
+// value of each BB in the CFG. The higher 32 bits are the CR32 of the numbers
+// of selects, indirect calls, mem ops and edges.

To make sure I understand: we use two CRCs instead of one CRC because we want 
to extract low/high 32 bits for debugging purposes?

Otherwise, we can just use one CRC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:642
 
   // Hash format for context sensitive profile. Reserve 4 bits for other
   // information.

davidxl wrote:
> The comment looks stale.
I think the 4 bits are still reserved for CSPGO. See 
https://reviews.llvm.org/rG6cdf3d8086fe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi updated this revision to Diff 281364.
yamauchi added a comment.
Herald added subscribers: cfe-commits, dexonsmith, steven_wu.
Herald added a project: clang.

Use JamCRC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

Files:
  clang/test/CodeGen/Inputs/thinlto_expect1.proftext
  clang/test/CodeGen/Inputs/thinlto_expect2.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap.proftext
  clang/test/CodeGenCXX/Inputs/profile-remap_entry.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR.proftext
  clang/test/Profile/Inputs/gcc-flag-compatibility_IR_entry.proftext
  compiler-rt/test/profile/Linux/instrprof-value-merge.c
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/PR41279.proftext
  llvm/test/Transforms/PGOProfile/Inputs/PR41279_2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch1_large_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/branch2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
  llvm/test/Transforms/PGOProfile/Inputs/criticaledge_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/cspgo.proftext
  llvm/test/Transforms/PGOProfile/Inputs/diag_no_value_sites.proftext
  llvm/test/Transforms/PGOProfile/Inputs/fix_entry_count.proftext
  llvm/test/Transforms/PGOProfile/Inputs/func_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirect_call.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr.proftext
  llvm/test/Transforms/PGOProfile/Inputs/indirectbr_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible.proftext
  llvm/test/Transforms/PGOProfile/Inputs/irreducible_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad.proftext
  llvm/test/Transforms/PGOProfile/Inputs/landingpad_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/large_count_remarks.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop1_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/loop2_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
  llvm/test/Transforms/PGOProfile/Inputs/noreturncall.proftext
  llvm/test/Transforms/PGOProfile/Inputs/remap.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select1.proftext
  llvm/test/Transforms/PGOProfile/Inputs/select2.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/switch_entry.proftext
  llvm/test/Transforms/PGOProfile/Inputs/thinlto_cs.proftext
  llvm/test/Transforms/PGOProfile/PR41279.ll
  llvm/test/Transforms/PGOProfile/PR41279_2.ll
  llvm/test/Transforms/PGOProfile/branch1.ll
  llvm/test/Transforms/PGOProfile/branch2.ll
  llvm/test/Transforms/PGOProfile/criticaledge.ll
  llvm/test/Transforms/PGOProfile/instr_entry_bb.ll
  llvm/test/Transforms/PGOProfile/landingpad.ll
  llvm/test/Transforms/PGOProfile/loop1.ll
  llvm/test/Transforms/PGOProfile/loop2.ll
  llvm/test/Transforms/PGOProfile/memop_size_from_strlen.ll
  llvm/test/Transforms/PGOProfile/single_bb.ll
  llvm/test/Transforms/PGOProfile/switch.ll

Index: llvm/test/Transforms/PGOProfile/switch.ll
===
--- llvm/test/Transforms/PGOProfile/switch.ll
+++ llvm/test/Transforms/PGOProfile/switch.ll
@@ -19,7 +19,7 @@
 entry:
 ; GEN: entry:
 ; NOTENTRY-NOT: call void @llvm.instrprof.increment
-; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 0)
+; ENTRY: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 519522107823649919, i32 4, i32 0)
   switch i32 %i, label %sw.default [
 i32 1, label %sw.bb
 i32 2, label %sw.bb1
@@ -31,23 +31,23 @@
 
 sw.bb:
 ; GEN: sw.bb:
-; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch, i32 0, i32 0), i64 46200943743, i32 4, i32 2)
+; GEN: call void @llvm.instrprof.increment(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @__profn_test_switch,