[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

This revision was automatically updated to reflect the committed changes. Closed by commit rGd768b97424f9: [Support] change StringMap hash function from djbHash to xxHash (authored by erikdesjardins). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins updated this revision to Diff 494861. erikdesjardins added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142862/new/ https://reviews.llvm.org/D142862 Files:

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/test/ObjectYAML/Offload/multiple_members.yaml:21 Value:"gfx908" Content: "cafefeed" Rebase:) I fixed obj2yaml Comment at:

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins updated this revision to Diff 494836. erikdesjardins edited the summary of this revision. erikdesjardins added a comment. update description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142862/new/ https://reviews.llvm.org/D142862

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins updated this revision to Diff 494835. erikdesjardins added a comment. rebase, remove now-unnecessary test changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142862/new/ https://reviews.llvm.org/D142862 Files:

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay added a comment. > `For posterity, the tests that fail on main are:` > > `Skipped : 43` > [...] You can remove these from the description. They are flaky tests unrelated to this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: mlir/test/Transforms/print-op-graph.mlir:1 // RUN: mlir-opt -allow-unregistered-dialect -mlir-elide-elementsattrs-if-larger=2 -view-op-graph %s -o %t

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins added inline comments. Comment at: mlir/test/Transforms/print-op-graph.mlir:9 // DFG: subgraph {{.*}} { -// DFG: v[[ARG0:.*]] [label = "arg0" -// DFG: v[[CONST10:.*]] [label ={{.*}}10 : i32 +// DFG:

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay added inline comments. Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:638 - EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h"))); + EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.h"), testPath("A.cc"))); // Make

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

serge-sans-paille added a comment. https://llvm-compile-time-tracker.com/compare.php?from=1a17739d3aa78599c32f6106e05dcfa7f3f9e823=9c1adc5fcca177d6628bdb22c72e7546c7b691aa=instructions:u looks good to me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins added a comment. In D142862#4089860 , @serge-sans-paille wrote: > Can you take a shot against https://llvm-compile-time-tracker.com/ so that > we get an hint of the practical speedup? I ran rustc perf tests using a patched LLVM in

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins updated this revision to Diff 494155. erikdesjardins added a comment. rebase over landed D142861 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142862/new/ https://reviews.llvm.org/D142862 Files:

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

goldstein.w.n added a comment. FWIW, if the only use if hashtables `xxhash` has a slight perf bug in `XXH3_len_129to240_128b` and `XXH3_len_129to240_64b` where it will repeat the last the block if length is a factor of `32`/`16` respectively. `XXH3_len_129to240_128b`: -

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay added a comment. To enable smooth hash function migration in the future, we can explore the idea of adding `#ifdef EXPENSIVE_CHECKS\nshuffle` (see https://github.com/llvm/llvm-project/issues/34483). That means we should fix these tests properly to be non-dependent on the iteration

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins added a subscriber: nikic. erikdesjardins added a comment. In D142862#4089860 , @serge-sans-paille wrote: > Can you take a shot against https://llvm-compile-time-tracker.com/ so that > we get an hint of the practical speedup? I don't

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

serge-sans-paille added a comment. Do you intend to (optionnaly) provide XXH3 as described in https://github.com/Cyan4973/xxHash ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142862/new/ https://reviews.llvm.org/D142862

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

serge-sans-paille added a comment. Can you take a shot against https://llvm-compile-time-tracker.com/ so that we get an hint of the practical speedup? Looks like xxhash.h is missing from the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins added inline comments. Comment at: llvm/test/DebugInfo/X86/gnu-public-names.ll:68-71 ; ASM: .byte 32 # Attributes: VARIABLE, EXTERNAL +; ASM-NEXT: .asciz "ns::global_namespace_variable" # External Name +; ASM: .byte 32

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

erikdesjardins created this revision. Herald added subscribers: Moerafaat, zero9178, Enna1, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, wenlei, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen,