[ExecutionEngine] Avoid repeated hash lookups (NFC) #132587
Conversation
kazutakahirata
deleted the
cleanup_001_repeated_hash_lookups_llvm_ExecutionEngine
branch
This reverts commit 0b181de.
Reverts #132587 Due to causing test failures on several of Linaro's buildbots. Several MLIR test failures and at least one test timing out. I doubt it's the patch itself, but instead an issue it has uncovered. Revert while we dig into that.
|
I've reverted this sooner than we might normally do, because it's intended to be a performance improvement so it's a clean revert. Going to get you details of the test failures... |
|
A normal AArch64 build: https://lab.llvm.org/buildbot/#/builders/84/builds/1436 Both got hit which means this isn't SVE specific. Lots of MLIR failures, that use And explains why no other platform had this problem. |
| @@ -1515,15 +1515,15 @@ void RuntimeDyldELF::resolveAArch64Branch(unsigned SectionID, | |||
| uint64_t Offset = RelI->getOffset(); | |||
| unsigned RelType = RelI->getType(); | |||
| // Look for an existing stub. | |||
| StubMap::const_iterator i = Stubs.find(Value); | |||
| if (i != Stubs.end()) { | |||
| auto [It, Inserted] = Stubs.try_emplace(Value); | |||
There was a problem hiding this comment.
I think the problem here is that before, we only added an entry to the map if resolveAArch64ShortBranch returned false. Now, it could return true and it's too late, we already added the entry to the map.
There was a problem hiding this comment.
@DavidSpickett Ah, I see. Without this patch, we sometimes don't insert a new entry depending on the outcome of resolveAArch64ShortBranch.
Thanks for the revert!
|
@kazutakahirata , I think this would reproduce by doing |
…)" (#133101) Reverts llvm/llvm-project#132587 Due to causing test failures on several of Linaro's buildbots. Several MLIR test failures and at least one test timing out. I doubt it's the patch itself, but instead an issue it has uncovered. Revert while we dig into that.
No description provided.