Skip to content

[TableGen] Avoid repeated hash lookups (NFC) #122586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

kazutakahirata
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Jan 11, 2025

@llvm/pr-subscribers-tablegen

Author: Kazu Hirata (kazutakahirata)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/122586.diff

1 Files Affected:

  • (modified) llvm/utils/TableGen/Common/CodeGenSchedule.cpp (+5-7)
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
index 06d82daebac0d5..7f4230affca09c 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
@@ -258,11 +258,9 @@ void CodeGenSchedModels::checkSTIPredicates() const {
   // There cannot be multiple declarations with the same name.
   for (const Record *R : Records.getAllDerivedDefinitions("STIPredicateDecl")) {
     StringRef Name = R->getValueAsString("Name");
-    const auto It = Declarations.find(Name);
-    if (It == Declarations.end()) {
-      Declarations[Name] = R;
+    const auto [It, Inserted] = Declarations.try_emplace(Name, R);
+    if (Inserted)
       continue;
-    }
 
     PrintError(R->getLoc(), "STIPredicate " + Name + " multiply declared.");
     PrintFatalNote(It->second->getLoc(), "Previous declaration was here.");
@@ -417,9 +415,9 @@ void CodeGenSchedModels::collectSTIPredicates() {
   for (const Record *R : Records.getAllDerivedDefinitions("STIPredicate")) {
     const Record *Decl = R->getValueAsDef("Declaration");
 
-    const auto It = Decl2Index.find(Decl);
-    if (It == Decl2Index.end()) {
-      Decl2Index[Decl] = STIPredicates.size();
+    const auto [It, Inserted] =
+        Decl2Index.try_emplace(Decl, STIPredicates.size());
+    if (Inserted) {
       STIPredicateFunction Predicate(Decl);
       Predicate.addDefinition(R);
       STIPredicates.emplace_back(std::move(Predicate));

@kazutakahirata kazutakahirata merged commit 07ff786 into llvm:main Jan 11, 2025
10 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_001_repeated_hash_TableGen_CodeGenSchedule branch January 11, 2025 21:15
BaiXilin pushed a commit to BaiXilin/llvm-fix-vnni-instr-types that referenced this pull request Jan 12, 2025
@mikaelholmen
Copy link
Collaborator

Hi @kazutakahirata

I've no idea what's going wrong but I noticed that the testcase
llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml
starts failing with this commit when I build with -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON.

If I then run

build-all-expensive/bin/llvm-lit -av test/tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml

It fails like

-- Testing: 1 tests, 1 workers --
FAIL: LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml (1 of 1)
******************** TEST 'LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Input file: /repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
Output file (aarch64): /repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
Loaded 3 functions from DWARF.
Loaded 3 functions from symbol table.
warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000001
addr=0x0000000000000248, file=  1, line=  5
addr=0x0000000000000254, file=  1, line=  7
addr=0x0000000000000258, file=  1, line=  9
addr=0x000000000000025c, file=  1, line=  8
addr=0x0000000000000260, file=  1, line= 11
addr=0x0000000000000264, file=  1, line= 10
addr=0x0000000000000268, file=  1, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
addr=0x0000000000000268, file=  2, line=  6


Pruned 5 functions, ended with 1 total
Duplicate address ranges with different debug info. occurred 2 time(s)

--
Command Output (stderr):
--
RUN: at line 1: /repo/uabelho/main-github/llvm/build-all-expensive/bin/yaml2obj /repo/uabelho/main-github/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml -o /repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
+ /repo/uabelho/main-github/llvm/build-all-expensive/bin/yaml2obj /repo/uabelho/main-github/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml -o /repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
RUN: at line 4: llvm-gsymutil --num-threads=1 --convert /repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM --out-file=/repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
+ llvm-gsymutil --num-threads=1 --convert /repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM --out-file=/repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
RUN: at line 5: llvm-gsymutil --verify --verbose /repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM | /repo/uabelho/main-github/llvm/build-all-expensive/bin/FileCheck --check-prefix=CHECK-GSYM-DEFAULT /repo/uabelho/main-github/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml
+ llvm-gsymutil --verify --verbose /repo/uabelho/main-github/llvm/build-all-expensive/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
+ /repo/uabelho/main-github/llvm/build-all-expensive/bin/FileCheck --check-prefix=CHECK-GSYM-DEFAULT /repo/uabelho/main-github/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml
/repo/uabelho/main-github/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml:16:23: error: CHECK-GSYM-DEFAULT: expected string not found in input
# CHECK-GSYM-DEFAULT: FunctionInfo @ 0x{{[0-9a-fA-F]+}}: [0x{{[0-9a-fA-F]+}} - 0x{{[0-9a-fA-F]+}}) "my_func_03"
                      ^
<stdin>:1:1: note: scanning from here
Header:
^
<stdin>:40:1: note: possible intended match here
FunctionInfo @ 0x000000bc: [0x0000000000000248 - 0x0000000000000270) "my_func_02"
^

Input file: <stdin>
Check file: /repo/uabelho/main-github/llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            1: Header: 
check:16'0     X~~~~~~~ error: no match found
            2:  Magic = 0x4753594d 
check:16'0     ~~~~~~~~~~~~~~~~~~~~
            3:  Version = 0x0001 
check:16'0     ~~~~~~~~~~~~~~~~~~
            4:  AddrOffSize = 0x02 
check:16'0     ~~~~~~~~~~~~~~~~~~~~
            5:  UUIDSize = 0x10 
check:16'0     ~~~~~~~~~~~~~~~~~
            6:  BaseAddress = 0x0000000000000000 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            .
            .
            .
           35: 0x00000030: "my_func_02" 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~
           36: 0x0000003b: "file_02.cpp" 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~
           37: 0x00000047: "my_func_03" 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~
           38: 0x00000052: "file_03.cpp" 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~
           39:  
check:16'0     ~
           40: FunctionInfo @ 0x000000bc: [0x0000000000000248 - 0x0000000000000270) "my_func_02" 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:16'1     ?                                                                                  possible intended match
           41: LineTable: 
check:16'0     ~~~~~~~~~~~
           42:  0x0000000000000248 /tmp/test_gsym_yaml/out/file_02.cpp:5 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           43:  0x0000000000000254 /tmp/test_gsym_yaml/out/file_02.cpp:7 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           44:  0x0000000000000258 /tmp/test_gsym_yaml/out/file_02.cpp:9 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           45:  0x000000000000025c /tmp/test_gsym_yaml/out/file_02.cpp:8 
check:16'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            .
            .
            .
>>>>>>

--

********************
********************
Failed Tests (1):
  LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml


Testing Time: 0.08s

Total Discovered Tests: 1
  Failed: 1 (100.00%)

@s-barannikov
Copy link
Contributor

The test is flaky. There have been several attempts to fix it, but apparently without success.
See git log on the test file.

kazutakahirata added a commit that referenced this pull request Jan 13, 2025
…)"

This partially reverts commit 07ff786.

The hunk being reverted in this patch seems to break:

  tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml

under LLVM_ENABLE_EXPENSIVE_CHECKS.
@kazutakahirata
Copy link
Contributor Author

@mikaelholmen I've reverted one of the two hunks to fix the failure. Honestly, I don't know how things break here.

@s-barannikov Ack on the flakiness. Thanks!

@mikaelholmen
Copy link
Collaborator

Ok, so the test is flaky and with that in mind I realize that it (sometimes) failed also before this patch.
Sorry for the noise I made here then. :(

kazutakahirata added a commit to kazutakahirata/llvm-project that referenced this pull request Jan 13, 2025
…122586)"

This partially reverts commit 07ff786.

The hunk being reverted in this patch seems to break:

  tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml

under LLVM_ENABLE_EXPENSIVE_CHECKS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants