Skip to content

Commit f5f6d89

Browse files
authored
Merge pull request swiftlang#25736 from ravikandhadai/oslog-optimization-string-builtin
2 parents 6bf46b9 + 7a3c6d5 commit f5f6d89

20 files changed

+341
-112
lines changed

include/swift/AST/Builtins.def

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,20 @@ BUILTIN_MISC_OPERATION(WillThrow, "willThrow", "", Special)
551551
/// poundAssert has type (Builtin.Int1, Builtin.RawPointer) -> ().
552552
BUILTIN_MISC_OPERATION(PoundAssert, "poundAssert", "", Special)
553553

554+
// BUILTIN_MISC_OPERATION_WITH_SILGEN - Miscellaneous operations that are
555+
// specially emitted during SIL generation.
556+
#ifndef BUILTIN_MISC_OPERATION_WITH_SILGEN
557+
#define BUILTIN_MISC_OPERATION_WITH_SILGEN(Id, Name, Attrs, Overload) \
558+
BUILTIN_MISC_OPERATION(Id, Name, Attrs, Overload)
559+
#endif
560+
561+
/// globalStringTablePointer has type String -> Builtin.RawPointer.
562+
/// It returns an immortal, global string table pointer for strings constructed
563+
/// from string literals.
564+
BUILTIN_MISC_OPERATION_WITH_SILGEN(GlobalStringTablePointer, "globalStringTablePointer", "", Special)
565+
566+
#undef BUILTIN_MISC_OPERATION_WITH_SILGEN
567+
554568
#undef BUILTIN_MISC_OPERATION
555569

556570
/// Builtins for instrumentation added by sanitizers during SILGen.

include/swift/AST/DiagnosticsSIL.def

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ NOTE(switch_value_case_doesnt_yield, none, "missing yield in the %0 case",
484484
NOTE(try_branch_doesnt_yield, none, "missing yield when error is "
485485
"%select{not |}0thrown", (bool))
486486

487-
// OS log optimization dianostics.
487+
// OS log optimization diagnostics.
488488

489489
ERROR(oslog_message_argument_not_found, none, "no argument of type %0 in "
490490
" the os log call", (Identifier))
@@ -505,6 +505,9 @@ ERROR(oslog_non_constant_interpolation, none, "'OSLogInterpolation' struct is "
505505
ERROR(oslog_property_not_constant, none, "'OSLogInterpolation.%0' is not a "
506506
"constant: formatting and privacy options must be literals", (StringRef))
507507

508+
ERROR(global_string_pointer_on_non_constant, none, "globalStringTablePointer "
509+
"builtin must used only on string literals", ())
510+
508511
#ifndef DIAG_NO_UNDEF
509512
# if defined(DIAG)
510513
# undef DIAG

lib/AST/Builtins.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,13 @@ static ValueDecl *getGetObjCTypeEncodingOperation(ASTContext &Context,
951951
return builder.build(Id);
952952
}
953953

954+
static ValueDecl *getGlobalStringTablePointer(ASTContext &Context,
955+
Identifier Id) {
956+
// String -> Builtin.RawPointer
957+
auto stringType = NominalType::get(Context.getStringDecl(), Type(), Context);
958+
return getBuiltinFunction(Id, {stringType}, Context.TheRawPointerType);
959+
}
960+
954961
static ValueDecl *getPoundAssert(ASTContext &Context, Identifier Id) {
955962
auto int1Type = BuiltinIntegerType::get(1, Context);
956963
auto optionalRawPointerType = BoundGenericEnumType::get(
@@ -1960,6 +1967,9 @@ ValueDecl *swift::getBuiltinValueDecl(ASTContext &Context, Identifier Id) {
19601967
case BuiltinValueKind::GetObjCTypeEncoding:
19611968
return getGetObjCTypeEncodingOperation(Context, Id);
19621969

1970+
case BuiltinValueKind::GlobalStringTablePointer:
1971+
return getGlobalStringTablePointer(Context, Id);
1972+
19631973
case BuiltinValueKind::PoundAssert:
19641974
return getPoundAssert(Context, Id);
19651975

lib/IRGen/GenBuiltin.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,18 @@ if (Builtin.ID == BuiltinValueKind::id) { \
354354
#define BUILTIN(ID, Name, Attrs) // Ignore the rest.
355355
#include "swift/AST/Builtins.def"
356356

357+
if (Builtin.ID == BuiltinValueKind::GlobalStringTablePointer) {
358+
// This builtin should be used only on strings constructed from a
359+
// string literal. If we ever get to the point of executing this builtin
360+
// at run time, it implies an incorrect use of the builtin and must result
361+
// in a trap.
362+
IGF.emitTrap(/*Unreachable=*/false);
363+
auto returnValue = llvm::UndefValue::get(IGF.IGM.Int8PtrTy);
364+
// Consume the arguments of the builtin.
365+
(void)args.claimAll();
366+
return out.add(returnValue);
367+
}
368+
357369
if (Builtin.ID == BuiltinValueKind::WillThrow) {
358370
// willThrow is emitted like a Swift function call with the error in
359371
// the error return register. We also have to pass a fake context

lib/SIL/OperandOwnership.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,16 @@ CONSTANT_OWNERSHIP_BUILTIN(Any, MustBeLive, Swift3ImplicitObjCEntrypoint)
10571057
CONSTANT_OWNERSHIP_BUILTIN(Any, MustBeLive, PoundAssert)
10581058
#undef CONSTANT_OWNERSHIP_BUILTIN
10591059

1060+
// CONSTANT_OWNERSHIP_BUILTIN(Owned, MustBeLive, GlobalStringTablePointer)
1061+
1062+
OperandOwnershipKindMap
1063+
OperandOwnershipKindBuiltinClassifier::visitGlobalStringTablePointer(
1064+
BuiltinInst *bi, StringRef attr) {
1065+
return Map::compatibilityMap(
1066+
{{ValueOwnershipKind::Guaranteed, UseLifetimeConstraint::MustBeLive},
1067+
{ValueOwnershipKind::Owned, UseLifetimeConstraint::MustBeLive}});
1068+
}
1069+
10601070
// Builtins that should be lowered to SIL instructions so we should never see
10611071
// them.
10621072
#define BUILTIN_SIL_OPERATION(ID, NAME, CATEGORY) \

lib/SIL/ValueOwnership.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ CONSTANT_OWNERSHIP_BUILTIN(Any, OnceWithContext)
491491
CONSTANT_OWNERSHIP_BUILTIN(Any, TSanInoutAccess)
492492
CONSTANT_OWNERSHIP_BUILTIN(Any, Swift3ImplicitObjCEntrypoint)
493493
CONSTANT_OWNERSHIP_BUILTIN(Any, PoundAssert)
494+
CONSTANT_OWNERSHIP_BUILTIN(Any, GlobalStringTablePointer)
494495

495496
#undef CONSTANT_OWNERSHIP_BUILTIN
496497

lib/SILGen/SILGenApply.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4057,12 +4057,14 @@ CallEmission::applySpecializedEmitter(SpecializedEmitter &specializedEmitter,
40574057
return firstLevelResult;
40584058
}
40594059

4060-
// Builtins.
4060+
// Named Builtins.
40614061
assert(specializedEmitter.isNamedBuiltin());
40624062
auto builtinName = specializedEmitter.getBuiltinName();
40634063
SmallVector<SILValue, 4> consumedArgs;
40644064
for (auto arg : uncurriedArgs) {
4065-
// Builtins have a special convention that takes everything at +1.
4065+
// Named builtins are by default assumed to take all arguments at +1 i.e.,
4066+
// as Owned or Trivial. Named builtins that don't follow this convention
4067+
// must use a specialized emitter.
40664068
auto maybePlusOne = arg.ensurePlusOne(SGF, uncurriedLoc.getValue());
40674069
consumedArgs.push_back(maybePlusOne.forward(SGF));
40684070
}

lib/SILGen/SILGenBuiltin.cpp

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,28 @@ static ManagedValue emitBuiltinTypeTrait(SILGenFunction &SGF,
10281028
return ManagedValue::forUnmanaged(val);
10291029
}
10301030

1031+
/// Emit SIL for the named builtin: globalStringTablePointer. Unlike the default
1032+
/// ownership convention for named builtins, which is to take (non-trivial)
1033+
/// arguments as Owned, this builtin accepts owned as well as guaranteed
1034+
/// arguments, and hence doesn't require the arguments to be at +1. Therefore,
1035+
/// this builtin is emitted specially.
1036+
static ManagedValue
1037+
emitBuiltinGlobalStringTablePointer(SILGenFunction &SGF, SILLocation loc,
1038+
SubstitutionMap subs,
1039+
ArrayRef<ManagedValue> args, SGFContext C) {
1040+
assert(args.size() == 1);
1041+
1042+
SILValue argValue = args[0].getValue();
1043+
auto &astContext = SGF.getASTContext();
1044+
Identifier builtinId = astContext.getIdentifier(
1045+
getBuiltinName(BuiltinValueKind::GlobalStringTablePointer));
1046+
1047+
auto resultVal = SGF.B.createBuiltin(loc, builtinId,
1048+
SILType::getRawPointerType(astContext),
1049+
subs, ArrayRef<SILValue>(argValue));
1050+
return SGF.emitManagedRValueWithCleanup(resultVal);
1051+
}
1052+
10311053
Optional<SpecializedEmitter>
10321054
SpecializedEmitter::forDecl(SILGenModule &SGM, SILDeclRef function) {
10331055
// Only consider standalone declarations in the Builtin module.
@@ -1052,6 +1074,7 @@ SpecializedEmitter::forDecl(SILGenModule &SGM, SILDeclRef function) {
10521074
#define BUILTIN(Id, Name, Attrs) \
10531075
case BuiltinValueKind::Id:
10541076
#define BUILTIN_SIL_OPERATION(Id, Name, Overload)
1077+
#define BUILTIN_MISC_OPERATION_WITH_SILGEN(Id, Name, Attrs, Overload)
10551078
#define BUILTIN_SANITIZER_OPERATION(Id, Name, Attrs)
10561079
#define BUILTIN_TYPE_CHECKER_OPERATION(Id, Name)
10571080
#define BUILTIN_TYPE_TRAIT_OPERATION(Id, Name)
@@ -1068,8 +1091,12 @@ SpecializedEmitter::forDecl(SILGenModule &SGM, SILDeclRef function) {
10681091
case BuiltinValueKind::Id: \
10691092
return SpecializedEmitter(&emitBuiltin##Id);
10701093

1071-
// Sanitizer builtins should never directly be called; they should only
1072-
// be inserted as instrumentation by SILGen.
1094+
#define BUILTIN_MISC_OPERATION_WITH_SILGEN(Id, Name, Attrs, Overload) \
1095+
case BuiltinValueKind::Id: \
1096+
return SpecializedEmitter(&emitBuiltin##Id);
1097+
1098+
// Sanitizer builtins should never directly be called; they should only
1099+
// be inserted as instrumentation by SILGen.
10731100
#define BUILTIN_SANITIZER_OPERATION(Id, Name, Attrs) \
10741101
case BuiltinValueKind::Id: \
10751102
llvm_unreachable("Sanitizer builtin called directly?");

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
}

lib/SILOptimizer/Transforms/AccessEnforcementReleaseSinking.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ static bool isBarrier(SILInstruction *inst) {
130130
case BuiltinValueKind::Swift3ImplicitObjCEntrypoint:
131131
case BuiltinValueKind::WillThrow:
132132
case BuiltinValueKind::PoundAssert:
133+
case BuiltinValueKind::GlobalStringTablePointer:
133134
return false;
134135

135136
// Handle some rare builtins that may be sensitive to object lifetime

lib/SILOptimizer/Utils/ConstantFolding.cpp

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1462,6 +1462,40 @@ bool ConstantFolder::constantFoldStringConcatenation(ApplyInst *AI) {
14621462
return true;
14631463
}
14641464

1465+
/// Given a buitin instruction calling globalStringTablePointer, check whether
1466+
/// the string passed to the builtin is constructed from a literal and if so,
1467+
/// replace the uses of the builtin instruction with the string_literal inst.
1468+
/// Otherwise, emit diagnostics if the function containing the builtin is not a
1469+
/// transparent function. Transparent functions will be handled in their
1470+
/// callers.
1471+
static bool
1472+
constantFoldGlobalStringTablePointerBuiltin(BuiltinInst *bi,
1473+
bool enableDiagnostics) {
1474+
// Look through string initializer to extract the string_literal instruction.
1475+
SILValue builtinOperand = bi->getOperand(0);
1476+
SILFunction *caller = bi->getFunction();
1477+
1478+
FullApplySite stringInitSite = FullApplySite::isa(builtinOperand);
1479+
if (!stringInitSite || !stringInitSite.getReferencedFunctionOrNull() ||
1480+
!stringInitSite.getReferencedFunctionOrNull()->hasSemanticsAttr(
1481+
"string.makeUTF8")) {
1482+
// Emit diagnostics only on non-transparent functions.
1483+
if (enableDiagnostics && !caller->isTransparent()) {
1484+
diagnose(caller->getASTContext(), bi->getLoc().getSourceLoc(),
1485+
diag::global_string_pointer_on_non_constant);
1486+
}
1487+
return false;
1488+
}
1489+
1490+
// Replace the builtin by the first argument of the "string.makeUTF8"
1491+
// initializer which must be a string_literal instruction.
1492+
SILValue stringLiteral = stringInitSite.getArgument(0);
1493+
assert(isa<StringLiteralInst>(stringLiteral));
1494+
1495+
bi->replaceAllUsesWith(stringLiteral);
1496+
return true;
1497+
}
1498+
14651499
/// Initialize the worklist to all of the constant instructions.
14661500
void ConstantFolder::initializeWorklist(SILFunction &F) {
14671501
for (auto &BB : F) {
@@ -1486,15 +1520,20 @@ void ConstantFolder::initializeWorklist(SILFunction &F) {
14861520
continue;
14871521
}
14881522

1489-
// Should we replace calls to assert_configuration by the assert
1490-
// configuration.
1523+
// - Should we replace calls to assert_configuration by the assert
1524+
// configuration and fold calls to any cond_unreachable.
14911525
if (AssertConfiguration != SILOptions::DisableReplacement &&
14921526
(isApplyOfBuiltin(I, BuiltinValueKind::AssertConf) ||
14931527
isApplyOfBuiltin(I, BuiltinValueKind::CondUnreachable))) {
14941528
WorkList.insert(&I);
14951529
continue;
14961530
}
14971531

1532+
if (isApplyOfBuiltin(I, BuiltinValueKind::GlobalStringTablePointer)) {
1533+
WorkList.insert(&I);
1534+
continue;
1535+
}
1536+
14981537
if (isa<CheckedCastBranchInst>(&I) ||
14991538
isa<CheckedCastAddrBranchInst>(&I) ||
15001539
isa<UnconditionalCheckedCastInst>(&I) ||
@@ -1642,6 +1681,19 @@ ConstantFolder::processWorkList() {
16421681
continue;
16431682
}
16441683

1684+
// Constant fold uses of globalStringTablePointer builtin.
1685+
if (isApplyOfBuiltin(*I, BuiltinValueKind::GlobalStringTablePointer)) {
1686+
if (constantFoldGlobalStringTablePointerBuiltin(cast<BuiltinInst>(I),
1687+
EnableDiagnostics)) {
1688+
// Here, the bulitin instruction got folded, so clean it up.
1689+
recursivelyDeleteTriviallyDeadInstructions(
1690+
I, /*force*/ true,
1691+
[&](SILInstruction *DeadI) { WorkList.remove(DeadI); });
1692+
InvalidateInstructions = true;
1693+
}
1694+
continue;
1695+
}
1696+
16451697
// Go through all users of the constant and try to fold them.
16461698
FoldedUsers.clear();
16471699
for (auto Result : I->getResults()) {

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+
}

0 commit comments

Comments
 (0)