Skip to content

Commit e4a29e1

Browse files
committed
Utils: Inhibit load/store folding through phis for llvm.protected.field.ptr.
Protected pointer field loads/stores should be paired with the intrinsic to avoid unnecessary address escapes. Pull Request: llvm#151649
1 parent acbf077 commit e4a29e1

File tree

5 files changed

+56
-8
lines changed

5 files changed

+56
-8
lines changed

llvm/include/llvm/Transforms/Utils/Local.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ LLVM_ABI bool EliminateDuplicatePHINodes(BasicBlock *BB);
182182
LLVM_ABI bool EliminateDuplicatePHINodes(BasicBlock *BB,
183183
SmallPtrSetImpl<PHINode *> &ToRemove);
184184

185+
/// Returns whether it is allowed and beneficial for optimizations to transform
186+
/// phi(load(ptr)) into load(phi(ptr)) or a similar transformation for stores.
187+
bool shouldFoldLoadStoreWithPointerOperandThroughPhi(const Value *Ptr);
188+
185189
/// This function is used to do simplification of a CFG. For example, it
186190
/// adjusts branches to branches to eliminate the extra hop, it eliminates
187191
/// unreachable basic blocks, and does other peephole optimization of the CFG.

llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -697,8 +697,7 @@ static bool isSafeAndProfitableToSinkLoad(LoadInst *L) {
697697
Instruction *InstCombinerImpl::foldPHIArgLoadIntoPHI(PHINode &PN) {
698698
LoadInst *FirstLI = cast<LoadInst>(PN.getIncomingValue(0));
699699

700-
// Can't forward swifterror through a phi.
701-
if (FirstLI->getOperand(0)->isSwiftError())
700+
if (!shouldFoldLoadStoreWithPointerOperandThroughPhi(FirstLI->getOperand(0)))
702701
return nullptr;
703702

704703
// FIXME: This is overconservative; this transform is allowed in some cases
@@ -737,8 +736,7 @@ Instruction *InstCombinerImpl::foldPHIArgLoadIntoPHI(PHINode &PN) {
737736
LI->getPointerAddressSpace() != LoadAddrSpace)
738737
return nullptr;
739738

740-
// Can't forward swifterror through a phi.
741-
if (LI->getOperand(0)->isSwiftError())
739+
if (!shouldFoldLoadStoreWithPointerOperandThroughPhi(LI->getOperand(0)))
742740
return nullptr;
743741

744742
// We can't sink the load if the loaded value could be modified between

llvm/lib/Transforms/Utils/Local.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3846,10 +3846,7 @@ bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {
38463846
if (Op->getType()->isMetadataTy())
38473847
return false;
38483848

3849-
// swifterror pointers can only be used by a load, store, or as a swifterror
3850-
// argument; swifterror pointers are not allowed to be used in select or phi
3851-
// instructions.
3852-
if (Op->isSwiftError())
3849+
if (!shouldFoldLoadStoreWithPointerOperandThroughPhi(Op))
38533850
return false;
38543851

38553852
// Cannot replace alloca argument with phi/select.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2134,6 +2134,22 @@ static bool replacingOperandWithVariableIsCheap(const Instruction *I,
21342134
return !isa<IntrinsicInst>(I);
21352135
}
21362136

2137+
bool llvm::shouldFoldLoadStoreWithPointerOperandThroughPhi(const Value *Ptr) {
2138+
// swifterror pointers can only be used by a load or store; sinking a load
2139+
// or store would require introducing a select for the pointer operand,
2140+
// which isn't allowed for swifterror pointers.
2141+
if (Ptr->isSwiftError())
2142+
return false;
2143+
2144+
// Protected pointer field loads/stores should be paired with the intrinsic
2145+
// to avoid unnecessary address escapes.
2146+
if (auto *II = dyn_cast<IntrinsicInst>(Ptr))
2147+
if (II->getIntrinsicID() == Intrinsic::protected_field_ptr)
2148+
return false;
2149+
2150+
return true;
2151+
}
2152+
21372153
// All instructions in Insts belong to different blocks that all unconditionally
21382154
// branch to a common successor. Analyze each instruction and return true if it
21392155
// would be possible to sink them into their successor, creating one common
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
; RUN: opt -O2 -S < %s | FileCheck %s
2+
3+
; Test that no optimization run at -O2 moves the loads into the exit block,
4+
; as this causes unnecessary address escapes with pointer field protection.
5+
6+
target triple = "aarch64-unknown-linux-gnu"
7+
8+
define ptr @phi_prot_ptr(i1 %sel, ptr %p1, ptr %p2) {
9+
br i1 %sel, label %t, label %f
10+
11+
; CHECK: t:
12+
t:
13+
; CHECK-NEXT: call
14+
%protp1 = call ptr @llvm.protected.field.ptr(ptr %p1, i64 1, i1 true)
15+
; CHECK-NEXT: load
16+
%load1 = load ptr, ptr %protp1
17+
br label %exit
18+
19+
; CHECK: f:
20+
f:
21+
; CHECK-NEXT: call
22+
%protp2 = call ptr @llvm.protected.field.ptr(ptr %p2, i64 2, i1 true)
23+
; CHECK-NEXT: load
24+
%load2 = load ptr, ptr %protp2
25+
br label %exit
26+
27+
; CHECK: exit:
28+
exit:
29+
; CHECK-NEXT: phi
30+
%retval = phi ptr [ %load1, %t ], [ %load2, %f ]
31+
; CHECK-NEXT: ret
32+
ret ptr %retval
33+
}

0 commit comments

Comments
 (0)