Skip to content

Commit 37be1f2

Browse files
gottesmmgottesmm
authored
Merge pull request swiftlang#38217 from gottesmm/pr-4acf599c8ec4a79b006ea0873129b68adb749de1
[assembly-vision] Change release to do backwards then forwards when inferring source locs.
2 parents 2464989 + 804bcac commit 37be1f2

File tree

7 files changed

+103
-9
lines changed

7 files changed

+103
-9
lines changed

‎benchmark/single-source/ChaCha.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ func checkResult(_ plaintext: [UInt8]) {
361361
}
362362

363363
@inline(never)
364+
@_assemblyVision
364365
public func run_ChaCha(_ N: Int) {
365366
let key = Array(repeating: UInt8(1), count: 32)
366367
let nonce = Array(repeating: UInt8(2), count: 12)

‎include/swift/SIL/OptimizationRemark.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ enum class SourceLocInferenceBehavior : unsigned {
153153
BackwardsThenForwards = BackwardScan | ForwardScan2nd,
154154
ForwardScanAlwaysInfer = ForwardScan | AlwaysInfer,
155155
BackwardScanAlwaysInfer = BackwardScan | AlwaysInfer,
156+
BackwardThenForwardAlwaysInfer = BackwardScan | ForwardScan2nd | AlwaysInfer,
156157
};
157158

158159
inline SourceLocInferenceBehavior operator&(SourceLocInferenceBehavior lhs,

‎lib/SIL/Utils/OptimizationRemark.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,19 @@ static SourceLoc getLocForPresentation(SILLocation loc,
161161
llvm_unreachable("covered switch");
162162
}
163163

164+
static bool instHasInferrableLoc(SILInstruction &inst) {
165+
if (isa<DeallocStackInst>(inst) || isa<StrongRetainInst>(inst) ||
166+
isa<StrongReleaseInst>(inst) || isa<RetainValueInst>(inst) ||
167+
isa<ReleaseValueInst>(inst) || isa<EndAccessInst>(inst))
168+
return false;
169+
170+
// Ignore empty tuples
171+
if (auto *tup = dyn_cast<TupleInst>(&inst))
172+
return tup->getNumOperands() != 0;
173+
174+
return true;
175+
}
176+
164177
/// The user has passed us an instruction that for some reason has a source loc
165178
/// that can not be used. Search down the current block for an instruction with
166179
/// a valid source loc and use that instead.
@@ -169,11 +182,16 @@ inferOptRemarkSearchForwards(SILInstruction &i,
169182
SourceLocPresentationKind presentationKind) {
170183
for (auto &inst :
171184
llvm::make_range(std::next(i.getIterator()), i.getParent()->end())) {
185+
if (!instHasInferrableLoc(inst))
186+
continue;
187+
// Skip instructions without a loc we care about since we move it around.
172188
auto newLoc = getLocForPresentation(inst.getLoc(), presentationKind);
173189
if (auto inlinedLoc = inst.getDebugScope()->getOutermostInlineLocation())
174190
newLoc = getLocForPresentation(inlinedLoc, presentationKind);
175-
if (newLoc.isValid())
191+
if (newLoc.isValid()) {
192+
LLVM_DEBUG(llvm::dbgs() << "Inferring loc from " << inst);
176193
return newLoc;
194+
}
177195
}
178196

179197
return SourceLoc();
@@ -188,11 +206,15 @@ inferOptRemarkSearchBackwards(SILInstruction &i,
188206
SourceLocPresentationKind presentationKind) {
189207
for (auto &inst : llvm::make_range(std::next(i.getReverseIterator()),
190208
i.getParent()->rend())) {
209+
if (!instHasInferrableLoc(inst))
210+
continue;
191211
auto loc = inst.getLoc();
192212
if (auto inlinedLoc = inst.getDebugScope()->getOutermostInlineLocation())
193213
loc = inlinedLoc;
194-
if (auto result = getLocForPresentation(loc, presentationKind))
214+
if (auto result = getLocForPresentation(loc, presentationKind)) {
215+
LLVM_DEBUG(llvm::dbgs() << "Inferring loc from " << inst);
195216
return result;
217+
}
196218
}
197219

198220
return SourceLoc();

‎lib/SILOptimizer/Transforms/AssemblyVisionRemarkGenerator.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,8 @@ void AssemblyVisionRemarkGeneratorInstructionVisitor::visitEndAccessInst(
604604
// Use the actual source loc of the begin_access if it works. Otherwise,
605605
// scan backwards.
606606
auto remark =
607-
RemarkMissed("memory", *eai, SourceLocInferenceBehavior::BackwardScan,
607+
RemarkMissed("memory", *eai,
608+
SourceLocInferenceBehavior::BackwardThenForwardAlwaysInfer,
608609
SourceLocPresentationKind::EndRange)
609610
<< "end exclusive access to value of type '"
610611
<< NV("ValueType", eai->getOperand()->getType()) << "'";
@@ -649,7 +650,7 @@ void AssemblyVisionRemarkGeneratorInstructionVisitor::visitStrongReleaseInst(
649650

650651
auto remark =
651652
RemarkMissed("memory", *sri,
652-
SourceLocInferenceBehavior::BackwardScanAlwaysInfer,
653+
SourceLocInferenceBehavior::BackwardThenForwardAlwaysInfer,
653654
SourceLocPresentationKind::EndRange)
654655
<< "release of type '" << NV("ValueType", sri->getOperand()->getType())
655656
<< "'";
@@ -693,7 +694,7 @@ void AssemblyVisionRemarkGeneratorInstructionVisitor::visitReleaseValueInst(
693694
// Releases end a lifetime scope so we infer scan backward.
694695
auto remark =
695696
RemarkMissed("memory", *rvi,
696-
SourceLocInferenceBehavior::BackwardScanAlwaysInfer)
697+
SourceLocInferenceBehavior::BackwardThenForwardAlwaysInfer)
697698
<< "release of type '" << NV("ValueType", rvi->getOperand()->getType())
698699
<< "'";
699700
for (auto arg : inferredArgs) {

‎test/SILOptimizer/assemblyvision_remark/basic.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ public func getGlobal() -> Klass {
1515
// expected-note @-5:12 {{of 'global'}}
1616
// expected-remark @-2:12 {{begin exclusive access to value of type 'Klass'}}
1717
// expected-note @-7:12 {{of 'global'}}
18+
// expected-remark @-4:12 {{end exclusive access to value of type 'Klass'}}
19+
// expected-note @-9:12 {{of 'global'}}
1820
}
1921

2022
// Make sure that the retain msg is at the beginning of the print and the

‎test/SILOptimizer/assemblyvision_remark/basic_yaml.swift

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public var global = Klass() // expected-remark {{heap allocated ref of type 'Kla
2828
// CHECK-NEXT: Pass: sil-assembly-vision-remark-gen
2929
// CHECK-NEXT: Name: sil.memory
3030
// CHECK-NEXT: DebugLoc: { File: '{{.*}}basic_yaml.swift',
31-
// CHECK-NEXT: Line: [[# @LINE + 27 ]], Column: 12 }
31+
// CHECK-NEXT: Line: [[# @LINE + 42 ]], Column: 12 }
3232
// CHECK-NEXT: Function: 'getGlobal()'
3333
// CHECK-NEXT: Args:
3434
// CHECK-NEXT: - String: 'begin exclusive access to value of type '''
@@ -42,6 +42,21 @@ public var global = Klass() // expected-remark {{heap allocated ref of type 'Kla
4242
// CHECK: --- !Missed
4343
// CHECK-NEXT: Pass: sil-assembly-vision-remark-gen
4444
// CHECK-NEXT: Name: sil.memory
45+
// CHECK-NEXT: DebugLoc: { File: '{{.*}}basic_yaml.swift',
46+
// CHECK-NEXT: Line: [[# @LINE + 27 ]], Column: 12 }
47+
// CHECK-NEXT: Function: 'getGlobal()'
48+
// CHECK-NEXT: Args:
49+
// CHECK-NEXT: - String: 'end exclusive access to value of type '''
50+
// CHECK-NEXT: - ValueType: Klass
51+
// CHECK-NEXT: - String: ''''
52+
// CHECK-NEXT: - InferredValue: 'of ''global'''
53+
// CHECK-NEXT: DebugLoc: { File: '{{.*}}basic_yaml.swift',
54+
// CHECK-NEXT: Line: [[# @LINE - 29 ]], Column: 12 }
55+
// CHECK-NEXT: ...
56+
//
57+
// CHECK: --- !Missed
58+
// CHECK-NEXT: Pass: sil-assembly-vision-remark-gen
59+
// CHECK-NEXT: Name: sil.memory
4560
// CHECK-NEXT: DebugLoc: { File: '{{.*}}basic_yaml.swift',
4661
// CHECK-NEXT: Line: [[# @LINE + 12]], Column: 5 }
4762
// CHECK-NEXT: Function: 'getGlobal()'
@@ -51,14 +66,17 @@ public var global = Klass() // expected-remark {{heap allocated ref of type 'Kla
5166
// CHECK-NEXT: - String: ''''
5267
// CHECK-NEXT: - InferredValue: 'of ''global'''
5368
// CHECK-NEXT: DebugLoc: { File: '{{.*}}basic_yaml.swift',
54-
// CHECK-NEXT: Line: [[# @LINE - 29 ]], Column: 12 }
69+
// CHECK-NEXT: Line: [[# @LINE - 44 ]], Column: 12 }
5570
// CHECK-NEXT: ...
5671
@inline(never)
5772
public func getGlobal() -> Klass {
5873
return global // expected-remark @:5 {{retain of type 'Klass'}}
59-
// expected-note @-34:12 {{of 'global'}}
74+
// expected-note @-49:12 {{of 'global'}}
6075
// expected-remark @-2 {{begin exclusive access to value of type 'Klass'}}
61-
// expected-note @-36:12 {{of 'global'}}
76+
// expected-note @-51:12 {{of 'global'}}
77+
// NOTE: We really want the end access at :18, not :12. TODO Fix this!
78+
// expected-remark @-5 {{end exclusive access to value of type 'Klass'}}
79+
// expected-note @-54:12 {{of 'global'}}
6280
}
6381

6482
// CHECK: --- !Missed

‎test/SILOptimizer/assemblyvision_remark/chacha.swift

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// RUN: %target-swiftc_driver -Osize -emit-sil %s -o /dev/null -Xfrontend -verify
2+
// REQUIRES: optimized_stdlib,swift_stdlib_no_asserts
3+
4+
// An extraction from the benchmark ChaCha20 that we were not ignoring
5+
// dealloc_stack and other end scope instructions.
6+
7+
enum ChaCha20 { }
8+
9+
extension ChaCha20 {
10+
@inline(never)
11+
public static func encrypt<Key: Collection, Nonce: Collection, Bytes: MutableCollection>(bytes: inout Bytes, key: Key, nonce: Nonce, initialCounter: UInt32 = 0) where Bytes.Element == UInt8, Key.Element == UInt8, Nonce.Element == UInt8 {
12+
print("I am lost...")
13+
}
14+
}
15+
16+
@inline(never)
17+
func checkResult(_ plaintext: [UInt8]) {
18+
precondition(plaintext.first! == 6 && plaintext.last! == 254)
19+
var hash: UInt64 = 0
20+
for byte in plaintext {
21+
// rotate
22+
hash = (hash &<< 8) | (hash &>> (64 - 8))
23+
hash ^= UInt64(byte)
24+
}
25+
precondition(hash == 0xa1bcdb217d8d14e4)
26+
}
27+
28+
@_semantics("optremark.sil-assembly-vision-remark-gen")
29+
public func run_ChaCha(_ N: Int) {
30+
let key = Array(repeating: UInt8(1), count: 32)
31+
let nonce = Array(repeating: UInt8(2), count: 12)
32+
33+
var checkedtext = Array(repeating: UInt8(0), count: 1024)
34+
ChaCha20.encrypt(bytes: &checkedtext, key: key, nonce: nonce)
35+
checkResult(checkedtext) // expected-remark {{release of type '}}
36+
// expected-note @-3 {{of 'checkedtext}}
37+
38+
var plaintext = Array(repeating: UInt8(0), count: 30720)
39+
for _ in 1...N {
40+
ChaCha20.encrypt(bytes: &plaintext, key: key, nonce: nonce)
41+
print(plaintext.first!) // expected-remark @:11 {{heap allocated ref of type '}}
42+
// expected-remark @-1:27 {{release of type '}}
43+
}
44+
} // expected-remark {{release of type '}}
45+
// expected-note @-7 {{of 'plaintext}}
46+
// expected-remark @-2 {{release of type '}}
47+
// expected-note @-16 {{of 'nonce}}
48+
// expected-remark @-4 {{release of type '}}
49+
// expected-note @-19 {{of 'key}}

0 commit comments

Comments
 (0)