[llvm] [Transforms] Speed up SSAUpdater::FindExistingPHI (PR #100281)
William Junda Huang via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 26 16:48:21 PDT 2024
https://github.com/huangjd updated https://github.com/llvm/llvm-project/pull/100281
>From 3b2ffc540c6c5a6794d6a49d3c6e08f154dc864a Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Tue, 23 Jul 2024 19:21:41 -0400
Subject: [PATCH 1/3] [Transforms] Speed up SSAUpdater::FindExistingPHI
In SSAUpdater::FindExistingPHI, the cleanup function is inefficient for
large function with many blocks because it clears the Phi value
reference for every block if not matched for every phi value, even if
most blocks are not modified by CheckIfPHIMatches. Updated the behavior
to only clear modified blocks.
---
.../llvm/Transforms/Utils/SSAUpdaterImpl.h | 24 ++++++++++++++-----
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
index 28ff6c4c7927d..24cbeaf72f82b 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
@@ -418,10 +418,6 @@ class SSAUpdaterImpl {
RecordMatchingPHIs(BlockList);
break;
}
- // Match failed: clear all the PHITag values.
- for (typename BlockListTy::iterator I = BlockList->begin(),
- E = BlockList->end(); I != E; ++I)
- (*I)->PHITag = nullptr;
}
}
@@ -429,10 +425,21 @@ class SSAUpdaterImpl {
/// in the BBMap.
bool CheckIfPHIMatches(PhiT *PHI) {
SmallVector<PhiT *, 20> WorkList;
+ SmallVector<BBInfo *, 20> TaggedBlocks;
WorkList.push_back(PHI);
+ // Match failed: clear all the PHITag values. Only need to clear visited
+ // blocks.
+ auto OnFalseCleanup = [&]() {
+ for (BBInfo *TaggedBlock : TaggedBlocks) {
+ TaggedBlock->PHITag = nullptr;
+ }
+ };
+
// Mark that the block containing this PHI has been visited.
- BBMap[PHI->getParent()]->PHITag = PHI;
+ BBInfo *PHIBlock = BBMap[PHI->getParent()];
+ PHIBlock->PHITag = PHI;
+ TaggedBlocks.push_back(PHIBlock);
while (!WorkList.empty()) {
PHI = WorkList.pop_back_val();
@@ -450,21 +457,26 @@ class SSAUpdaterImpl {
if (PredInfo->AvailableVal) {
if (IncomingVal == PredInfo->AvailableVal)
continue;
+ OnFalseCleanup();
return false;
}
// Check if the value is a PHI in the correct block.
PhiT *IncomingPHIVal = Traits::ValueIsPHI(IncomingVal, Updater);
- if (!IncomingPHIVal || IncomingPHIVal->getParent() != PredInfo->BB)
+ if (!IncomingPHIVal || IncomingPHIVal->getParent() != PredInfo->BB) {
+ OnFalseCleanup();
return false;
+ }
// If this block has already been visited, check if this PHI matches.
if (PredInfo->PHITag) {
if (IncomingPHIVal == PredInfo->PHITag)
continue;
+ OnFalseCleanup();
return false;
}
PredInfo->PHITag = IncomingPHIVal;
+ TaggedBlocks.push_back(PredInfo);
WorkList.push_back(IncomingPHIVal);
}
>From 8cc062dda28502666f4549dea6a5d82bee7ce4c2 Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Wed, 24 Jul 2024 17:40:28 -0400
Subject: [PATCH 2/3] Use scope_exit for more organized code
---
.../llvm/Transforms/Utils/SSAUpdaterImpl.h | 26 +++++++++----------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
index 24cbeaf72f82b..078ee0088e344 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
@@ -15,6 +15,7 @@
#define LLVM_TRANSFORMS_UTILS_SSAUPDATERIMPL_H
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Debug.h"
@@ -413,8 +414,9 @@ class SSAUpdaterImpl {
/// FindExistingPHI - Look through the PHI nodes in a block to see if any of
/// them match what is needed.
void FindExistingPHI(BlkT *BB, BlockListTy *BlockList) {
+ SmallVector<BBInfo *, 20> TaggedBlocks;
for (auto &SomePHI : BB->phis()) {
- if (CheckIfPHIMatches(&SomePHI)) {
+ if (CheckIfPHIMatches(&SomePHI, TaggedBlocks)) {
RecordMatchingPHIs(BlockList);
break;
}
@@ -423,18 +425,18 @@ class SSAUpdaterImpl {
/// CheckIfPHIMatches - Check if a PHI node matches the placement and values
/// in the BBMap.
- bool CheckIfPHIMatches(PhiT *PHI) {
- SmallVector<PhiT *, 20> WorkList;
- SmallVector<BBInfo *, 20> TaggedBlocks;
- WorkList.push_back(PHI);
-
+ bool CheckIfPHIMatches(PhiT *PHI, SmallVector<BBInfo *, 20> &TaggedBlocks) {
// Match failed: clear all the PHITag values. Only need to clear visited
// blocks.
- auto OnFalseCleanup = [&]() {
+ auto Cleanup = make_scope_exit([&]() {
for (BBInfo *TaggedBlock : TaggedBlocks) {
TaggedBlock->PHITag = nullptr;
}
- };
+ TaggedBlocks.clear();
+ });
+
+ SmallVector<PhiT *, 20> WorkList;
+ WorkList.push_back(PHI);
// Mark that the block containing this PHI has been visited.
BBInfo *PHIBlock = BBMap[PHI->getParent()];
@@ -457,22 +459,18 @@ class SSAUpdaterImpl {
if (PredInfo->AvailableVal) {
if (IncomingVal == PredInfo->AvailableVal)
continue;
- OnFalseCleanup();
return false;
}
// Check if the value is a PHI in the correct block.
PhiT *IncomingPHIVal = Traits::ValueIsPHI(IncomingVal, Updater);
- if (!IncomingPHIVal || IncomingPHIVal->getParent() != PredInfo->BB) {
- OnFalseCleanup();
+ if (!IncomingPHIVal || IncomingPHIVal->getParent() != PredInfo->BB)
return false;
- }
// If this block has already been visited, check if this PHI matches.
if (PredInfo->PHITag) {
if (IncomingPHIVal == PredInfo->PHITag)
continue;
- OnFalseCleanup();
return false;
}
PredInfo->PHITag = IncomingPHIVal;
@@ -481,6 +479,8 @@ class SSAUpdaterImpl {
WorkList.push_back(IncomingPHIVal);
}
}
+ // Match found, keep PHITags.
+ Cleanup.release();
return true;
}
>From 5fc4bd58bbba528833931d5a5a203a5ab58d5a3b Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Fri, 26 Jul 2024 19:48:01 -0400
Subject: [PATCH 3/3] Coding style fix
---
llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
index 078ee0088e344..010d6b0de9f6a 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h
@@ -425,13 +425,12 @@ class SSAUpdaterImpl {
/// CheckIfPHIMatches - Check if a PHI node matches the placement and values
/// in the BBMap.
- bool CheckIfPHIMatches(PhiT *PHI, SmallVector<BBInfo *, 20> &TaggedBlocks) {
+ bool CheckIfPHIMatches(PhiT *PHI, SmallVectorImpl<BBInfo *> &TaggedBlocks) {
// Match failed: clear all the PHITag values. Only need to clear visited
// blocks.
auto Cleanup = make_scope_exit([&]() {
- for (BBInfo *TaggedBlock : TaggedBlocks) {
+ for (BBInfo *TaggedBlock : TaggedBlocks)
TaggedBlock->PHITag = nullptr;
- }
TaggedBlocks.clear();
});
More information about the llvm-commits
mailing list