Skip to content

Commit

Permalink
VN: don't propagate values across protected regions
Browse files Browse the repository at this point in the history
Contributes to #33
  • Loading branch information
dubiousconst282 committed May 9, 2024
1 parent 94d9b59 commit 1572f2a
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 2 deletions.
3 changes: 3 additions & 0 deletions src/DistIL/Analysis/ProtectedRegionAnalysis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,7 @@ public IEnumerable<BasicBlock> GetExitBlocks()
}
return null;
}

/// <summary> If this is a handler region, returns the guard defining it. </summary>
public GuardInst? GetHandlerGuard() => StartBlock.Users().OfType<GuardInst>().FirstOrDefault();
}
2 changes: 1 addition & 1 deletion src/DistIL/CodeGen/Cil/LayoutedCFG.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ AbsRange LayoutRegion(ProtectedRegion node)
foreach (var child in node.Children) {
PlaceAntecessorBlocks(child.StartBlock);
var range = LayoutRegion(child);
var guard = child.StartBlock.Users().OfType<GuardInst>().FirstOrDefault();
var guard = child.GetHandlerGuard();

if (guard != null) {
clauses[clauseIndices[guard]].UpdateRanges(child.StartBlock, range);
Expand Down
2 changes: 1 addition & 1 deletion src/DistIL/IR/Utils/IRVerifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private void CheckTerminator(BasicBlock block, ProtectedRegionAnalysis regionAna
if (region == regionAnalysis.Root) {
Error(block, "Cannot leave or resume from root region");
} else if (block.Last is ResumeInst resume) {
var guard = region.StartBlock.Users().OfType<GuardInst>().FirstOrDefault();
var guard = region.GetHandlerGuard();

if (guard == null) {
Error(block, "Missing guard for filter or finally region");
Expand Down
9 changes: 9 additions & 0 deletions src/DistIL/Passes/ValueNumbering.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public MethodPassResult Run(MethodTransformContext ctx)
DomTree = ctx.GetAnalysis<DominatorTree>(),
AliasAnalysis = ctx.GetAnalysis<AliasAnalysis>(),
FuncInfo = ctx.Compilation.GetAnalysis<GlobalFunctionEffects>(),
RegionAnalysis = ctx.GetAnalysis<ProtectedRegionAnalysis>(),
};
bool madeChanges = false;

Expand All @@ -36,6 +37,7 @@ class ValueTable
public required DominatorTree DomTree;
public required AliasAnalysis AliasAnalysis;
public required GlobalFunctionEffects FuncInfo;
public required ProtectedRegionAnalysis RegionAnalysis;

public bool Process(Instruction inst)
{
Expand Down Expand Up @@ -70,6 +72,13 @@ public bool CheckAvail(Instruction def, Instruction user)
// Trivial reject
if (!DomTree.Dominates(def.Block, user.Block)) return false;

// Don't propagate values across different regions. #33
// This check is a bit conservative, there are cases where
// it is safe to propagate values into child regions.
var defRegion = RegionAnalysis.GetBlockRegion(def.Block);
var useRegion = RegionAnalysis.GetBlockRegion(user.Block);
if (defRegion != useRegion) return false;

// Trivial accept instructions that don't access memory or are pure
if (!(def is MemoryInst or CallInst)) return true;

Expand Down

0 comments on commit 1572f2a

Please sign in to comment.