This revision was automatically updated to reflect the committed changes.
Closed by commit rL318229: [PGO] Detect more structural changes with the stable
hash (authored by vedantk).
Changed prior to commit:
https://reviews.llvm.org/D39446?vs=122368&id=122939#toc
Repository:
rL LLVM
https://
vsk added a comment.
Thanks for the review Alex. There hasn't been any more feedback so I'll commit
this soon.
https://reviews.llvm.org/D39446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
arphaman added a comment.
ok, sgtm
https://reviews.llvm.org/D39446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vsk added inline comments.
Comment at: lib/CodeGen/CodeGenPGO.cpp:244
+ DEFINE_NESTABLE_TRAVERSAL(CXXTryStmt)
+ DEFINE_NESTABLE_TRAVERSAL(CXXCatchStmt)
+
arphaman wrote:
> What about `ObjCAtTryStmt`?
I thought about this, but: we don't place a counter on ObjC a
arphaman added a comment.
Generally it LG
Comment at: lib/CodeGen/CodeGenPGO.cpp:244
+ DEFINE_NESTABLE_TRAVERSAL(CXXTryStmt)
+ DEFINE_NESTABLE_TRAVERSAL(CXXCatchStmt)
+
What about `ObjCAtTryStmt`?
Comment at: lib/CodeGen/CodeGenPGO.cpp:321
vsk updated this revision to Diff 122368.
vsk edited the summary of this revision.
vsk added a comment.
- Consider logical nots and some binary comparison operators in the hash, per
an offline conversation with @bogner
https://reviews.llvm.org/D39446
Files:
lib/CodeGen/CodeGenPGO.cpp
test/
vsk updated this revision to Diff 121224.
vsk marked 2 inline comments as done.
vsk edited the summary of this revision.
vsk added a comment.
- Handle loop nesting, conditions, and out-of-order control flow.
- Improve test coverage. Add a format compatibility test, and check that
functions which
vsk planned changes to this revision.
vsk added a comment.
Apart from Alex's point, this patch is missing a compatibility test for v5 of
the profile format, and is missing coverage for the new entries in `HashType`.
Comment at: lib/CodeGen/CodeGenPGO.cpp:186
+if (Hash.getH
bob.wilson added a comment.
I'm excited to see some progress on this, but since there is overhead to adding
a new hashing scheme, I think we should do more before introducing a new
scheme. One of the problems with the previous scheme is that is did not take
into account nesting. Distinguishing
arphaman added inline comments.
Comment at: lib/CodeGen/CodeGenPGO.cpp:186
+if (Hash.getHashVersion() != PGO_HASH_V1)
+ Type = getHashType(Hash.getHashVersion(), S);
+
Am I missing something or will this call always return the same type as before
since
vsk created this revision.
Lifting from Bob Wilson's notes: The hash value that we compute and store in
PGO profile data to detect out-of-date profiles does not include enough
information. This means that many significant changes to the source will not
cause compiler warnings about the profile bei
11 matches
Mail list logo
