Merge pull request #32911 from guitard0g/oslog_profiling

[Constant Evaluator] Skip code-coverage instrumentation during constant evaluation

Author: Josh Learn

include/swift/AST/Builtins.def

@@ -701,6 +701,9 @@ BUILTIN_MISC_OPERATION(PoundAssert, "poundAssert", "", Special)
 // TypePtrAuthDiscriminator has type <T> (T.Type) -> Int64
 BUILTIN_MISC_OPERATION(TypePtrAuthDiscriminator, "typePtrAuthDiscriminator", "n", Special)
 
+//  int_instrprof_increment has type (Builtin.RawPointer, Builtin.Int64, Builtin.Int32, Builtin.Int32) -> ().
+BUILTIN_MISC_OPERATION(IntInstrprofIncrement, "int_instrprof_increment", "", Special)
+
 // BUILTIN_MISC_OPERATION_WITH_SILGEN - Miscellaneous operations that are
 // specially emitted during SIL generation.
 //

include/swift/SIL/InstructionUtils.h

@@ -118,6 +118,10 @@ bool mayCheckRefCount(SILInstruction *User);
 /// run-time sanitizers.
 bool isSanitizerInstrumentation(SILInstruction *Instruction);
 
+/// Return true when the instruction represents added instrumentation for
+/// run-time sanitizers or code coverage.
+bool isInstrumentation(SILInstruction *Instruction);
+
 /// Check that this is a partial apply of a reabstraction thunk and return the
 /// argument of the partial apply if it is.
 SILValue isPartialApplyOfReabstractionThunk(PartialApplyInst *PAI);

lib/AST/Builtins.cpp

@@ -20,6 +20,7 @@
 #include "swift/AST/Module.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/TypeCheckRequests.h"
+#include "swift/AST/Types.h"
 #include "swift/Strings.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -950,6 +951,15 @@ static ValueDecl *getUnsafeGuaranteedEnd(ASTContext &C, Identifier Id) {
   return getBuiltinFunction(Id, { Int8Ty }, TupleType::getEmpty(C));
 }
 
+static ValueDecl *getIntInstrprofIncrement(ASTContext &C, Identifier Id) {
+  // (Builtin.RawPointer, Builtin.Int64, Builtin.Int32, Builtin.Int32) -> ()
+  Type Int64Ty = BuiltinIntegerType::get(64, C);
+  Type Int32Ty = BuiltinIntegerType::get(32, C);
+  return getBuiltinFunction(Id,
+                            {C.TheRawPointerType, Int64Ty, Int32Ty, Int32Ty},
+                            TupleType::getEmpty(C));
+}
+
 static ValueDecl *getTypePtrAuthDiscriminator(ASTContext &C, Identifier Id) {
   // <T : AnyObject> (T.Type) -> Int64
   BuiltinFunctionBuilder builder(C);
@@ -2458,6 +2468,9 @@ ValueDecl *swift::getBuiltinValueDecl(ASTContext &Context, Identifier Id) {
                               {},
                               TupleType::getEmpty(Context));
 
+  case BuiltinValueKind::IntInstrprofIncrement:
+    return getIntInstrprofIncrement(Context, Id);
+
   case BuiltinValueKind::TypePtrAuthDiscriminator:
     return getTypePtrAuthDiscriminator(Context, Id);
 

lib/SIL/IR/OperandOwnership.cpp

@@ -1013,6 +1013,7 @@ ANY_OWNERSHIP_BUILTIN(Swift3ImplicitObjCEntrypoint)
 ANY_OWNERSHIP_BUILTIN(PoundAssert)
 ANY_OWNERSHIP_BUILTIN(GlobalStringTablePointer)
 ANY_OWNERSHIP_BUILTIN(TypePtrAuthDiscriminator)
+ANY_OWNERSHIP_BUILTIN(IntInstrprofIncrement)
 #undef ANY_OWNERSHIP_BUILTIN
 
 // This is correct today since we do not have any builtins which return

lib/SIL/IR/ValueOwnership.cpp

@@ -539,6 +539,7 @@ CONSTANT_OWNERSHIP_BUILTIN(None, TSanInoutAccess)
 CONSTANT_OWNERSHIP_BUILTIN(None, Swift3ImplicitObjCEntrypoint)
 CONSTANT_OWNERSHIP_BUILTIN(None, PoundAssert)
 CONSTANT_OWNERSHIP_BUILTIN(None, TypePtrAuthDiscriminator)
+CONSTANT_OWNERSHIP_BUILTIN(None, IntInstrprofIncrement)
 CONSTANT_OWNERSHIP_BUILTIN(None, GlobalStringTablePointer)
 
 #undef CONSTANT_OWNERSHIP_BUILTIN

lib/SIL/Utils/InstructionUtils.cpp

@@ -327,6 +327,21 @@ bool swift::isSanitizerInstrumentation(SILInstruction *Instruction) {
   return false;
 }
 
+// Instrumentation instructions should not affect the correctness of the
+// program. That is, they should not affect the observable program state.
+// The constant evaluator relies on this property to skip instructions.
+bool swift::isInstrumentation(SILInstruction *Instruction) {
+  if (isSanitizerInstrumentation(Instruction))
+    return true;
+
+  if (BuiltinInst *bi = dyn_cast<BuiltinInst>(Instruction)) {
+    if (bi->getBuiltinKind() == BuiltinValueKind::IntInstrprofIncrement)
+      return true;
+  }
+
+  return false;
+}
+
 SILValue swift::isPartialApplyOfReabstractionThunk(PartialApplyInst *PAI) {
   // A partial_apply of a reabstraction thunk either has a single capture
   // (a function) or two captures (function and dynamic Self type).

lib/SILGen/SILGenFunction.cpp

@@ -973,8 +973,10 @@ void SILGenFunction::emitProfilerIncrement(ASTNode N) {
       B.createIntegerLiteral(Loc, Int64Ty, SP->getPGOFuncHash()),
       B.createIntegerLiteral(Loc, Int32Ty, SP->getNumRegionCounters()),
       B.createIntegerLiteral(Loc, Int32Ty, CounterIt->second)};
-  B.createBuiltin(Loc, C.getIdentifier("int_instrprof_increment"),
-                  SGM.Types.getEmptyTupleType(), {}, Args);
+  B.createBuiltin(
+      Loc,
+      C.getIdentifier(getBuiltinName(BuiltinValueKind::IntInstrprofIncrement)),
+      SGM.Types.getEmptyTupleType(), {}, Args);
 }
 
 ProfileCounter SILGenFunction::loadProfilerCount(ASTNode Node) const {

lib/SILOptimizer/Transforms/AccessEnforcementReleaseSinking.cpp

@@ -139,6 +139,7 @@ static bool isBarrier(SILInstruction *inst) {
     case BuiltinValueKind::TypePtrAuthDiscriminator:
     case BuiltinValueKind::GlobalStringTablePointer:
     case BuiltinValueKind::COWBufferForReading:
+    case BuiltinValueKind::IntInstrprofIncrement:
       return false;
 
     // Handle some rare builtins that may be sensitive to object lifetime

lib/SILOptimizer/Utils/ConstExpr.cpp

@@ -10,8 +10,10 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "swift/SIL/SILInstruction.h"
 #define DEBUG_TYPE "ConstExpr"
 #include "swift/SILOptimizer/Utils/ConstExpr.h"
+#include "swift/AST/Builtins.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/SemanticAttrs.h"
 #include "swift/AST/SubstitutionMap.h"
@@ -1756,8 +1758,8 @@ ConstExprFunctionState::evaluateFlowSensitive(SILInstruction *inst) {
       isa<ReleaseValueInst>(inst) || isa<StrongRetainInst>(inst) ||
       isa<StrongReleaseInst>(inst) || isa<DestroyValueInst>(inst) ||
       isa<EndBorrowInst>(inst) ||
-      // Skip sanitizer instrumentation
-      isSanitizerInstrumentation(inst))
+      // Skip instrumentation
+      isInstrumentation(inst))
     return None;
 
   // If this is a special flow-sensitive instruction like a stack allocation,

test/SILOptimizer/Inputs/OSLogConstantEvaluable.swift

@@ -0,0 +1,47 @@
+// Source Input used by OSLogConstantEvaluableTest.swift and OSLogConstantEvaluableProfilerTest.swift
+// Test that the functions defined in the OSLogPrototype overlay annotated as
+// constant evaluable are so (with the constexpr-limit defined above).
+// This test is meant to catch regressions in the OSLog overlay implementation
+// affecting the constant evaluability of functions that are expected to be so.
+
+import OSLogTestHelper
+
+// CHECK-LABEL: @init(stringLiteral: String) -> OSLogMessage
+// CHECK-NOT: error:
+@_semantics("test_driver")
+func osLogMessageStringLiteralInitTest() -> OSLogMessage {
+  return "A string literal"
+}
+
+// CHECK-LABEL: @init(literalCapacity: Int, interpolationCount: Int) -> OSLogInterpolation
+// CHECK-NOT: error:
+// CHECK-LABEL: @appendLiteral(String) -> ()
+// CHECK-NOT: error:
+// CHECK-LABEL: @appendInterpolation(_: @autoclosure () -> Int, format: OSLogIntegerFormatting, align: OSLogStringAlignment, privacy: OSLogPrivacy) -> ()
+// CHECK-NOT: error:
+// CHECK-LABEL: @appendLiteral(String) -> ()
+// CHECK-NOT: error:
+// CHECK-LABEL: @init(stringInterpolation: OSLogInterpolation) -> OSLogMessage
+// CHECK-NOT: error:
+@_semantics("test_driver")
+func intValueInterpolationTest() -> OSLogMessage {
+  return "An integer value \(10)"
+}
+
+// CHECK-LABEL: @init(literalCapacity: Int, interpolationCount: Int) -> OSLogInterpolation
+// CHECK-NOT: error:
+// CHECK-LABEL: @appendInterpolation(_: @autoclosure () -> String, align: OSLogStringAlignment, privacy: OSLogPrivacy) -> ()
+// CHECK-NOT: error:
+@_semantics("test_driver")
+func stringValueInterpolationTest() -> OSLogMessage {
+  return "A string value \("xyz")"
+}
+
+@_semantics("test_driver")
+func intValueWithPrecisionTest() -> OSLogMessage {
+  return
+    """
+     An integer value \
+     \(10, format: .decimal(minDigits: 25), align: .right(columns: 10))
+    """
+}