Skip to content

Commit 7a3c6d5

Browse files
committed
[stdlib][oslog] Create an SPI _globalStringTablePointer that exposes
the builtin.globalStringTablePointer to the new OSLog overlay. Modify the new OSLog implementation to use this SPI instead of `withCString` to pass the (compiler-generated) format string to the C os_log_impl ABI. Move the OSLogOptimization pass before constant propagation in the pass pipeline so that the SPI and the builtin it uses can be folded to a string_literal instruction. Update OSLogTests to work with the changes in the implementation.
1 parent 37f524e commit 7a3c6d5

File tree

8 files changed

+154
-104
lines changed

8 files changed

+154
-104
lines changed

lib/SILOptimizer/Mandatory/OSLogOptimization.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
/// arguments are packed. This pass is closely tied to the implementation of
2323
/// the log APIs.
2424
///
25-
/// Pass Dependencies: MandatoryInlining. This pass also uses
26-
/// `ConstExprStepEvaluator` defined in `Utils/ConstExpr.cpp`.
25+
/// Pass Dependencies: This pass depends on MandatoryInlining and Mandatory
26+
/// Linking happening before this pass and ConstantPropagation happening after
27+
/// this pass. This pass also uses `ConstExprStepEvaluator` defined in
28+
/// `Utils/ConstExpr.cpp`.
2729
///
2830
/// Algorithm Overview:
2931
///

lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,13 @@ static void addMandatoryOptPipeline(SILPassPipelinePlan &P) {
114114
// SSA based diagnostics.
115115
P.addPredictableMemoryAccessOptimizations();
116116

117+
// This phase performs optimizations necessary for correct interoperation of
118+
// Swift os log APIs with C os_log ABIs.
119+
// Pass dependencies: this pass depends on MandatoryInlining and Mandatory
120+
// Linking happening before this pass and ConstantPropagation happening after
121+
// this pass.
122+
P.addOSLogOptimization();
123+
117124
// Diagnostic ConstantPropagation must be rerun on deserialized functions
118125
// because it is sensitive to the assert configuration.
119126
// Consequently, certain optimization passes beyond this point will also rerun.
@@ -129,10 +136,6 @@ static void addMandatoryOptPipeline(SILPassPipelinePlan &P) {
129136
P.addYieldOnceCheck();
130137
P.addEmitDFDiagnostics();
131138

132-
// This phase performs optimizations necessary for correct interoperation of
133-
// Swift os log APIs with C os_log ABIs.
134-
P.addOSLogOptimization();
135-
136139
// Canonical swift requires all non cond_br critical edges to be split.
137140
P.addSplitNonCondBrCriticalEdges();
138141
}

stdlib/private/OSLog/OSLog.swift

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,32 +66,31 @@ internal func osLog(
6666
let preamble = message.interpolation.preamble
6767
let argumentCount = message.interpolation.argumentCount
6868
let bufferSize = message.bufferSize
69+
let formatStringPointer = _getGlobalStringTablePointer(formatString)
6970

7071
// Code that will execute at runtime.
71-
let arguments = message.interpolation.arguments
72-
formatString.withCString { cFormatString in
72+
guard logObject.isEnabled(type: logLevel) else { return }
7373

74-
guard logObject.isEnabled(type: logLevel) else { return }
74+
let arguments = message.interpolation.arguments
7575

76-
// Ideally, we could stack allocate the buffer as it is local to this
77-
// function and also its size is a compile-time constant.
78-
let bufferMemory =
79-
UnsafeMutablePointer<UInt8>.allocate(capacity: bufferSize)
80-
var builder = OSLogByteBufferBuilder(bufferMemory)
76+
// Ideally, we could stack allocate the buffer as it is local to this
77+
// function and also its size is a compile-time constant.
78+
let bufferMemory =
79+
UnsafeMutablePointer<UInt8>.allocate(capacity: bufferSize)
80+
var builder = OSLogByteBufferBuilder(bufferMemory)
8181

82-
builder.serialize(preamble)
83-
builder.serialize(argumentCount)
84-
arguments.serialize(into: &builder)
82+
builder.serialize(preamble)
83+
builder.serialize(argumentCount)
84+
arguments.serialize(into: &builder)
8585

86-
___os_log_impl(UnsafeMutableRawPointer(mutating: #dsohandle),
87-
logObject,
88-
logLevel,
89-
cFormatString,
90-
bufferMemory,
91-
UInt32(bufferSize))
86+
___os_log_impl(UnsafeMutableRawPointer(mutating: #dsohandle),
87+
logObject,
88+
logLevel,
89+
formatStringPointer,
90+
bufferMemory,
91+
UInt32(bufferSize))
9292

93-
bufferMemory.deallocate()
94-
}
93+
bufferMemory.deallocate()
9594
}
9695

9796
/// A test helper that constructs a byte buffer and a format string from an

stdlib/private/OSLog/OSLogMessage.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public var maxOSLogArgumentCount: UInt8 { return 48 }
4848

4949
@usableFromInline
5050
@_transparent
51-
internal var bitsPerByte: Int { return 8 }
51+
internal var logBitsPerByte: Int { return 3 }
5252

5353
/// Represents a string interpolation passed to the log APIs.
5454
///
@@ -458,7 +458,7 @@ internal struct OSLogSerializationInfo {
458458
@usableFromInline
459459
@_transparent
460460
internal static func sizeForEncoding(_ type: Int.Type) -> Int {
461-
return Int.bitWidth / bitsPerByte
461+
return Int.bitWidth &>> logBitsPerByte
462462
}
463463
}
464464

stdlib/public/core/Builtin.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -965,3 +965,14 @@ public func _openExistential<ExistentialType, ContainedType, ResultType>(
965965
Builtin.unreachable()
966966
}
967967

968+
/// Given a string that is constructed from a string literal, return a pointer
969+
/// to the global string table location that contains the string literal.
970+
/// This function will trap when it is invoked on strings that are not
971+
/// constructed from literals or if the construction site of the string is not
972+
/// in the function containing the call to this SPI.
973+
@_transparent
974+
@_alwaysEmitIntoClient
975+
public // @SPI(OSLog)
976+
func _getGlobalStringTablePointer(_ constant: String) -> UnsafePointer<CChar> {
977+
return UnsafePointer<CChar>(Builtin.globalStringTablePointer(constant));
978+
}

test/IRGen/builtins.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,7 @@ throw MyError.A
809809
// CHECK-LABEL: define {{.*}}globalStringTablePointer
810810
// CHECK: call void @llvm.trap()
811811
// CHECK: ret i8* undef
812+
@_transparent
812813
func globalStringTablePointerUse(_ str: String) -> Builtin.RawPointer {
813814
return Builtin.globalStringTablePointer(str);
814815
}

test/SILOptimizer/OSLogPrototypeCompileDiagnostics.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,25 @@ if #available(OSX 10.12, iOS 10.0, watchOS 3.0, tvOS 10.0, *) {
1515
// FIXME: log APIs must always be passed a string interpolation literal.
1616
// Diagnose this.
1717
h.log(level: .debug, message)
18+
// expected-error @-1 {{globalStringTablePointer builtin must used only on string literals}}
1819
}
1920

2021
func testNonconstantFormatOption(h: Logger, formatOpt: IntFormat) {
2122
h.log(level: .debug, "Minimum integer value: \(Int.min, format: formatOpt)")
2223
// expected-error @-1 {{'OSLogInterpolation.formatString' is not a constant: formatting and privacy options must be literals}}
24+
// expected-error @-2 {{globalStringTablePointer builtin must used only on string literals}}
2325
}
2426

2527
func testNonconstantPrivacyOption(h: Logger, privacyOpt: Privacy) {
2628
h.log(level: .debug, "Minimum integer value: \(Int.min, privacy: privacyOpt)")
2729
// expected-error @-1 {{'OSLogInterpolation.formatString' is not a constant: formatting and privacy options must be literals}}
30+
// expected-error @-2 {{globalStringTablePointer builtin must used only on string literals}}
2831
}
2932

30-
// FIXME: the following two tests should produce diagnostics and are not
33+
// FIXME: the following test should produce diagnostics and is a not
3134
// valid uses of the log APIs. The string interpolation passed to the os log
3235
// call must be apart of the log call, it cannot be constructed earlier.
36+
// It nonetheless works fine, but should be rejected.
3337
func testNoninlinedOSLogMessage(h: Logger) {
3438
let logMessage: OSLogMessage = "Minimum integer value: \(Int.min)"
3539
h.log(level: .debug, logMessage)
@@ -41,6 +45,7 @@ if #available(OSX 10.12, iOS 10.0, watchOS 3.0, tvOS 10.0, *) {
4145
return;
4246
}
4347
h.log(level: .debug, logMessage)
48+
// expected-error @-1 {{globalStringTablePointer builtin must used only on string literals}}
4449
}
4550
}
4651

0 commit comments

Comments
 (0)