[llvm] [NewGVN] Fix caching for OpIsSafeForPhiOfOps (PR #98340)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 10 14:25:12 PDT 2024
https://github.com/ManuelJBrito updated https://github.com/llvm/llvm-project/pull/98340
>From f108e30c15cf7aef77ca4c083a1cbf6fd553a76e Mon Sep 17 00:00:00 2001
From: ManuelJBrito <manuel.brito at tecnico.ulisboa.pt>
Date: Wed, 10 Jul 2024 22:24:51 +0100
Subject: [PATCH] Add block identifier to cache key
---
llvm/lib/Transforms/Scalar/NewGVN.cpp | 23 ++--
.../Transforms/NewGVN/cache-safe-phiofops.ll | 100 ++++++++++++++++++
2 files changed, 116 insertions(+), 7 deletions(-)
create mode 100644 llvm/test/Transforms/NewGVN/cache-safe-phiofops.ll
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index 4cba196ed688a..dcbb7a8cc190e 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -529,7 +529,11 @@ class NewGVN {
// IR.
SmallPtrSet<const Instruction *, 8> PHINodeUses;
- DenseMap<const Value *, bool> OpSafeForPHIOfOps;
+ // The cached results, in general, are only valid for the specific block where
+ // they were computed. The unsigned part of the key is a unique block
+ // identifier
+ DenseMap<std::pair<const Value *, unsigned>, bool> OpSafeForPHIOfOps;
+ unsigned CacheIdx;
// Map a temporary instruction we created to a parent block.
DenseMap<const Value *, BasicBlock *> TempToBlock;
@@ -2600,19 +2604,19 @@ bool NewGVN::OpIsSafeForPHIOfOps(Value *V, const BasicBlock *PHIBlock,
if (!isa<Instruction>(I))
continue;
- auto OISIt = OpSafeForPHIOfOps.find(I);
+ auto OISIt = OpSafeForPHIOfOps.find({I, CacheIdx});
if (OISIt != OpSafeForPHIOfOps.end())
return OISIt->second;
// Keep walking until we either dominate the phi block, or hit a phi, or run
// out of things to check.
if (DT->properlyDominates(getBlockForValue(I), PHIBlock)) {
- OpSafeForPHIOfOps.insert({I, true});
+ OpSafeForPHIOfOps.insert({{I, CacheIdx}, true});
continue;
}
// PHI in the same block.
if (isa<PHINode>(I) && getBlockForValue(I) == PHIBlock) {
- OpSafeForPHIOfOps.insert({I, false});
+ OpSafeForPHIOfOps.insert({{I, CacheIdx}, false});
return false;
}
@@ -2631,10 +2635,10 @@ bool NewGVN::OpIsSafeForPHIOfOps(Value *V, const BasicBlock *PHIBlock,
if (!isa<Instruction>(Op))
continue;
// Stop now if we find an unsafe operand.
- auto OISIt = OpSafeForPHIOfOps.find(OrigI);
+ auto OISIt = OpSafeForPHIOfOps.find({OrigI, CacheIdx});
if (OISIt != OpSafeForPHIOfOps.end()) {
if (!OISIt->second) {
- OpSafeForPHIOfOps.insert({I, false});
+ OpSafeForPHIOfOps.insert({{I, CacheIdx}, false});
return false;
}
continue;
@@ -2644,7 +2648,7 @@ bool NewGVN::OpIsSafeForPHIOfOps(Value *V, const BasicBlock *PHIBlock,
Worklist.push_back(cast<Instruction>(Op));
}
}
- OpSafeForPHIOfOps.insert({V, true});
+ OpSafeForPHIOfOps.insert({{V, CacheIdx}, true});
return true;
}
@@ -3297,6 +3301,7 @@ void NewGVN::verifyIterationSettled(Function &F) {
TouchedInstructions.set();
TouchedInstructions.reset(0);
OpSafeForPHIOfOps.clear();
+ CacheIdx = 0;
iterateTouchedInstructions();
DenseSet<std::pair<const CongruenceClass *, const CongruenceClass *>>
EqualClasses;
@@ -3400,6 +3405,8 @@ void NewGVN::iterateTouchedInstructions() {
<< " because it is unreachable\n");
continue;
}
+ // Use the appropriate cache for "OpIsSafeForPHIOfOps".
+ CacheIdx = RPOOrdering.lookup(DT->getNode(CurrBlock)) - 1;
updateProcessedCount(CurrBlock);
}
// Reset after processing (because we may mark ourselves as touched when
@@ -3479,6 +3486,8 @@ bool NewGVN::runGVN() {
LLVM_DEBUG(dbgs() << "Block " << getBlockName(&F.getEntryBlock())
<< " marked reachable\n");
ReachableBlocks.insert(&F.getEntryBlock());
+ // Use index corresponding to entry block.
+ CacheIdx = 0;
iterateTouchedInstructions();
verifyMemoryCongruency();
diff --git a/llvm/test/Transforms/NewGVN/cache-safe-phiofops.ll b/llvm/test/Transforms/NewGVN/cache-safe-phiofops.ll
new file mode 100644
index 0000000000000..d26d3f1190c24
--- /dev/null
+++ b/llvm/test/Transforms/NewGVN/cache-safe-phiofops.ll
@@ -0,0 +1,100 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=newgvn < %s | FileCheck %s
+
+define i32 @unsafe(i1 %arg, i32 %arg1) {
+; CHECK-LABEL: define i32 @unsafe(
+; CHECK-SAME: i1 [[ARG:%.*]], i32 [[ARG1:%.*]]) {
+; CHECK-NEXT: [[BB:.*:]]
+; CHECK-NEXT: br label %[[BB4:.*]]
+; CHECK: [[BB4]]:
+; CHECK-NEXT: br i1 [[ARG]], label %[[BB6:.*]], label %[[BB5:.*]]
+; CHECK: [[BB5]]:
+; CHECK-NEXT: br label %[[BB6]]
+; CHECK: [[BB6]]:
+; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[ARG1]], %[[BB5]] ], [ 0, %[[BB4]] ]
+; CHECK-NEXT: [[SREM:%.*]] = srem i32 [[ARG1]], [[PHI]]
+; CHECK-NEXT: store i32 [[SREM]], ptr null, align 4
+; CHECK-NEXT: br i1 [[ARG]], label %[[BB8:.*]], label %[[BB7:.*]]
+; CHECK: [[BB7]]:
+; CHECK-NEXT: br i1 true, label %[[BB8]], label %[[BB5]]
+; CHECK: [[BB8]]:
+; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i32 [ [[ARG1]], %[[BB6]] ], [ 0, %[[BB7]] ]
+; CHECK-NEXT: [[PHI9:%.*]] = phi i32 [ 0, %[[BB7]] ], [ 1, %[[BB6]] ]
+; CHECK-NEXT: store i32 [[PHIOFOPS]], ptr null, align 4
+; CHECK-NEXT: br label %[[BB4]]
+;
+bb:
+ br label %bb4
+
+bb4: ; preds = %bb8, %bb
+ br i1 %arg, label %bb6, label %bb5
+
+bb5: ; preds = %bb7, %bb4
+ br label %bb6
+
+bb6: ; preds = %bb5, %bb4
+ %phi = phi i32 [ %arg1, %bb5 ], [ 0, %bb4 ]
+ %or = or i32 %phi, %arg1
+ %srem = srem i32 %or, %phi
+ store i32 %srem, ptr null, align 4
+ br i1 %arg, label %bb8, label %bb7
+
+bb7: ; preds = %bb6
+ br i1 true, label %bb8, label %bb5
+
+bb8: ; preds = %bb7, %bb6
+ %phi9 = phi i32 [ 0, %bb7 ], [ 1, %bb6 ]
+ %mul = mul i32 %phi9, %or
+ store i32 %mul, ptr null, align 4
+ br label %bb4
+}
+
+define i32 @unsafe_load(i1 %arg) {
+; CHECK-LABEL: define i32 @unsafe_load(
+; CHECK-SAME: i1 [[ARG:%.*]]) {
+; CHECK-NEXT: [[BB:.*:]]
+; CHECK-NEXT: br label %[[BB1:.*]]
+; CHECK: [[BB1]]:
+; CHECK-NEXT: br i1 [[ARG]], label %[[BB3:.*]], label %[[BB2:.*]]
+; CHECK: [[BB2]]:
+; CHECK-NEXT: br label %[[BB3]]
+; CHECK: [[BB3]]:
+; CHECK-NEXT: [[PHI4:%.*]] = phi i32 [ 1, %[[BB2]] ], [ 0, %[[BB1]] ]
+; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr null, align 4
+; CHECK-NEXT: [[SREM:%.*]] = srem i32 [[LOAD]], [[PHI4]]
+; CHECK-NEXT: store i32 [[SREM]], ptr null, align 4
+; CHECK-NEXT: br i1 [[ARG]], label %[[BB6:.*]], label %[[BB5:.*]]
+; CHECK: [[BB5]]:
+; CHECK-NEXT: br i1 true, label %[[BB6]], label %[[BB2]]
+; CHECK: [[BB6]]:
+; CHECK-NEXT: [[PHIOFOPS:%.*]] = phi i32 [ [[LOAD]], %[[BB3]] ], [ 0, %[[BB5]] ]
+; CHECK-NEXT: [[PHI7:%.*]] = phi i32 [ 0, %[[BB5]] ], [ 1, %[[BB3]] ]
+; CHECK-NEXT: store i32 [[PHIOFOPS]], ptr null, align 4
+; CHECK-NEXT: br label %[[BB1]]
+;
+bb:
+ br label %bb1
+
+bb1: ; preds = %bb6, %bb
+ br i1 %arg, label %bb3, label %bb2
+
+bb2: ; preds = %bb5, %bb1
+ %phi = phi i32 [ 1, %bb1 ], [ 0, %bb5 ]
+ br label %bb3
+
+bb3: ; preds = %bb2, %bb1
+ %phi4 = phi i32 [ %phi, %bb2 ], [ 0, %bb1 ]
+ %load = load i32, ptr null, align 4
+ %srem = srem i32 %load, %phi4
+ store i32 %srem, ptr null, align 4
+ br i1 %arg, label %bb6, label %bb5
+
+bb5: ; preds = %bb3
+ br i1 true, label %bb6, label %bb2
+
+bb6: ; preds = %bb5, %bb3
+ %phi7 = phi i32 [ 0, %bb5 ], [ 1, %bb3 ]
+ %mul = mul i32 %phi7, %load
+ store i32 %mul, ptr null, align 4
+ br label %bb1
+}
More information about the llvm-commits
mailing list