[OSLog][Test] Update the new oslog overlay implementation to use

@_semantics("constant_evaluable") annotation to denote constant
evaluable functions.

Add a test suite that uses the sil-opt pass ConstantEvaluableSubsetChecker.cpp
to check the constant evaluability of function in the OSLog
overlay.

Author: ravikandhadai

include/swift/AST/DiagnosticsSIL.def

@@ -402,9 +402,11 @@ NOTE(constexpr_unsupported_instruction_found, none,
 NOTE(constexpr_unsupported_instruction_found_here,none, "operation"
      "%select{| used by this call is}0 not supported by the evaluator", (bool))
 
-NOTE(constexpr_unknown_function_called, none,
-    "encountered call to '%0' whose body is not available", (StringRef))
-NOTE(constexpr_unknown_function_called_here, none,
+NOTE(constexpr_found_callee_with_no_body, none,
+    "encountered call to '%0' whose body is not available. "
+    "Imported functions must be marked '@inlinable' to constant evaluate.",
+    (StringRef))
+NOTE(constexpr_callee_with_no_body, none,
     "%select{|calls a }0function whose body is not available", (bool))
 
 NOTE(constexpr_untracked_sil_value_use_found, none,
@@ -428,7 +430,10 @@ NOTE(constexpr_returned_by_unevaluated_instruction,none,
      "return value of an unevaluated instruction is not a constant", ())
 NOTE(constexpr_mutated_by_unevaluated_instruction,none, "value mutable by an "
     "unevaluated instruction is not a constant", ())
+
 ERROR(not_constant_evaluable, none, "not constant evaluable", ())
+ERROR(constexpr_imported_func_not_onone, none, "imported constant evaluable "
+      "function '%0' must be annotated '@_optimize(none)'", (StringRef))
 
 ERROR(non_physical_addressof,none,
       "addressof only works with purely physical lvalues; "

lib/SIL/SILConstants.cpp

@@ -20,7 +20,7 @@ using namespace swift;
 
 namespace swift {
 llvm::cl::opt<unsigned>
-    ConstExprLimit("constexpr-limit", llvm::cl::init(512),
+    ConstExprLimit("constexpr-limit", llvm::cl::init(1024),
                    llvm::cl::desc("Number of instructions interpreted in a"
                                   " constexpr function"));
 }
@@ -711,10 +711,10 @@ void SymbolicValue::emitUnknownDiagnosticNotes(SILLocation fallbackLoc) {
     SILFunction *callee = unknownReason.getCalleeWithoutImplmentation();
     std::string demangledCalleeName =
         Demangle::demangleSymbolAsString(callee->getName());
-    diagnose(ctx, diagLoc, diag::constexpr_unknown_function_called,
+    diagnose(ctx, diagLoc, diag::constexpr_found_callee_with_no_body,
              StringRef(demangledCalleeName));
     if (emitTriggerLocInDiag)
-      diagnose(ctx, triggerLoc, diag::constexpr_unknown_function_called_here,
+      diagnose(ctx, triggerLoc, diag::constexpr_callee_with_no_body,
                triggerLocSkipsInternalLocs);
     return;
   }

lib/SILOptimizer/Mandatory/OSLogOptimization.cpp

@@ -249,7 +249,7 @@ static bool shouldAttemptEvaluation(SILInstruction *inst) {
     return false;
 
   return calleeFun->hasSemanticsAttrThatStartsWith("string.") ||
-         calleeFun->hasSemanticsAttrThatStartsWith("oslog.");
+         calleeFun->hasSemanticsAttr("constant_evaluable");
 }
 
 /// Skip or evaluate the given instruction based on the evaluation policy and

lib/SILOptimizer/UtilityPasses/ConstantEvaluableSubsetChecker.cpp

@@ -65,13 +65,18 @@ class ConstantEvaluableSubsetChecker : public SILModuleTransform {
     SymbolicValueBumpAllocator allocator;
     ConstExprStepEvaluator stepEvaluator(allocator, fun,
                                          /*trackCallees*/ true);
+    bool previousEvaluationHadFatalError = false;
 
     for (auto currI = fun->getEntryBlock()->begin();;) {
       auto *inst = &(*currI);
 
       if (isa<ReturnInst>(inst))
         break;
 
+      assert(!previousEvaluationHadFatalError &&
+             "cannot continue evaluation of test driver as previous call "
+             "resulted in non-skippable evaluation error.");
+
       auto *applyInst = dyn_cast<ApplyInst>(inst);
       SILFunction *callee = nullptr;
       if (applyInst) {
@@ -85,7 +90,11 @@ class ConstantEvaluableSubsetChecker : public SILModuleTransform {
           !callee->hasSemanticsAttr(constantEvaluableSemanticsAttr)) {
         std::tie(nextInstOpt, errorVal) =
             stepEvaluator.tryEvaluateOrElseMakeEffectsNonConstant(currI);
-        assert(nextInstOpt && "non-constant control flow in the test driver");
+        if (!nextInstOpt) {
+          // This indicates an error in the test driver.
+          errorVal->emitUnknownDiagnosticNotes(inst->getLoc());
+          assert(false && "non-constant control flow in the test driver");
+        }
         currI = nextInstOpt.getValue();
         continue;
       }
@@ -101,17 +110,40 @@ class ConstantEvaluableSubsetChecker : public SILModuleTransform {
         errorVal->emitUnknownDiagnosticNotes(inst->getLoc());
       }
 
-      if (!nextInstOpt) {
-        break; // This instruction should be the end of the test driver.
+      if (nextInstOpt) {
+        currI = nextInstOpt.getValue();
+        continue;
       }
-      currI = nextInstOpt.getValue();
+
+      // Here, a non-skippable error like "instruction-limit exceeded" has been
+      // encountered during evaluation. Proceed to the next instruction but
+      // ensure that an assertion failure occurs if there is any instruction
+      // to evaluate after this instruction.
+      ++currI;
+      previousEvaluationHadFatalError = true;
     }
 
-    // Record functions that were called during this evaluation to detect
-    // whether the test drivers in the SILModule cover all function annotated
-    // as "constant_evaluable".
+    // For every function seen during the evaluation of this constant evaluable
+    // function:
+    // 1. Record it so as to detect whether the test drivers in the SILModule
+    // cover all function annotated as "constant_evaluable".
+    //
+    // 2. If the callee is annotated as constant_evaluable and is imported from
+    // a different Swift module (other than stdlib), check that the function is
+    // marked as Onone. Otherwise, it could have been optimized, which will
+    // break constant evaluability.
     for (SILFunction *callee : stepEvaluator.getFuncsCalledDuringEvaluation()) {
       evaluatedFunctions.insert(callee);
+
+      SILModule &calleeModule = callee->getModule();
+      if (callee->isAvailableExternally() &&
+          callee->hasSemanticsAttr(constantEvaluableSemanticsAttr) &&
+          callee->getOptimizationMode() != OptimizationMode::NoOptimization) {
+        diagnose(calleeModule.getASTContext(),
+                 callee->getLocation().getSourceLoc(),
+                 diag::constexpr_imported_func_not_onone,
+                 demangleSymbolName(callee->getName()));
+      }
     }
   }
 

lib/SILOptimizer/Utils/ConstExpr.cpp

@@ -1534,8 +1534,17 @@ ConstExprFunctionState::evaluateInstructionAndGetNext(
     assert(value.getKind() == SymbolicValue::Enum ||
            value.getKind() == SymbolicValue::EnumWithPayload);
 
-    auto *caseBB = switchInst->getCaseDestination(value.getEnumValue());
+    SILBasicBlock *caseBB =
+        switchInst->getCaseDestination(value.getEnumValue());
+    if (caseBB->getNumArguments() == 0)
+      return {caseBB->begin(), None};
+
     // Set up the arguments.
+
+    // When there are multiple payload components, they form a single
+    // tuple-typed argument.
+    assert(caseBB->getNumArguments() == 1);
+
     if (caseBB->getParent()->hasOwnership() &&
         switchInst->getDefaultBBOrNull() == caseBB) {
       // If we are visiting the default block and we are in ossa, then we may
@@ -1545,13 +1554,7 @@ ConstExprFunctionState::evaluateInstructionAndGetNext(
       return {caseBB->begin(), None};
     }
 
-    if (caseBB->getNumArguments() == 0)
-      return {caseBB->begin(), None};
-
     assert(value.getKind() == SymbolicValue::EnumWithPayload);
-    // When there are multiple payload components, they form a single
-    // tuple-typed argument.
-    assert(caseBB->getNumArguments() == 1);
     auto argument = value.getEnumPayloadValue();
     assert(argument.isConstant());
     setValue(caseBB->getArgument(0), argument);

stdlib/private/OSLog/OSLogIntegerTypes.swift

@@ -106,7 +106,7 @@ extension OSLogInterpolation {
   /// This function must be constant evaluable and all its arguments
   /// must be known at compile time.
   @inlinable
-  @_semantics("oslog.interpolation.getFormatSpecifier")
+  @_semantics("constant_evaluable")
   @_effects(readonly)
   @_optimize(none)
   internal func getIntegerFormatSpecifier<T>(
@@ -150,7 +150,7 @@ extension OSLogArguments {
 /// Return the number of bytes needed for serializing an integer argument as
 /// specified by os_log. This function must be constant evaluable.
 @inlinable
-@_semantics("oslog.integers.sizeForEncoding")
+@_semantics("constant_evaluable")
 @_effects(readonly)
 @_optimize(none)
 internal func sizeForEncoding<T>(

stdlib/private/OSLog/OSLogMessage.swift

@@ -95,7 +95,7 @@ public struct OSLogInterpolation : StringInterpolationProtocol {
   /// argument header. The first two bits are used to indicate privacy and
   /// the other two are reserved.
   @usableFromInline
-  @_frozen
+  @frozen
   internal enum ArgumentFlag {
     case privateFlag
     case publicFlag
@@ -117,7 +117,7 @@ public struct OSLogInterpolation : StringInterpolationProtocol {
   /// (Note that an auto-generated rawValue is not constant evaluable because
   /// it cannot be annotated so.)
   @usableFromInline
-  @_frozen
+  @frozen
   internal enum ArgumentType {
     case scalar, count, string, pointer, object
 
@@ -147,7 +147,7 @@ public struct OSLogInterpolation : StringInterpolationProtocol {
   /// mask indicate whether there is an argument that is private, and whether
   /// there is an argument that is non-scalar: String, NSObject or Pointer.
   @usableFromInline
-  @_frozen
+  @frozen
   internal enum PreambleBitMask {
     case privateBitMask
     case nonScalarBitMask
@@ -195,7 +195,7 @@ public struct OSLogInterpolation : StringInterpolationProtocol {
   /// An internal initializer that should be used only when there are no
   /// interpolated expressions. This function must be constant evaluable.
   @inlinable
-  @_semantics("oslog.interpolation.init")
+  @_semantics("constant_evaluable")
   @_optimize(none)
   internal init() {
     formatString = ""
@@ -216,7 +216,7 @@ public struct OSLogInterpolation : StringInterpolationProtocol {
   /// Return true if and only if the parameter is .private.
   /// This function must be constant evaluable.
   @inlinable
-  @_semantics("oslog.interpolation.isPrivate")
+  @_semantics("constant_evaluable")
   @_effects(readonly)
   @_optimize(none)
   internal func isPrivate(_ privacy: Privacy) -> Bool {
@@ -233,7 +233,7 @@ public struct OSLogInterpolation : StringInterpolationProtocol {
   /// of the header byte, respectively.
   /// This function should be constant evaluable.
   @inlinable
-  @_semantics("oslog.interpolation.getArgumentHeader")
+  @_semantics("constant_evaluable")
   @_effects(readonly)
   @_optimize(none)
   internal func getArgumentHeader(
@@ -248,7 +248,7 @@ public struct OSLogInterpolation : StringInterpolationProtocol {
   /// Compute the new preamble based whether the current argument is private
   /// or not. This function must be constant evaluable.
   @inlinable
-  @_semantics("oslog.interpolation.getUpdatedPreamble")
+  @_semantics("constant_evaluable")
   @_effects(readonly)
   @_optimize(none)
   internal func getUpdatedPreamble(
@@ -268,7 +268,8 @@ public struct OSLogInterpolation : StringInterpolationProtocol {
 
 extension String {
   /// Replace all percents "%" in the string by "%%" so that the string can be
-  /// interpreted as a C format string.
+  /// interpreted as a C format string. This function is constant evaluable
+  /// and its semantics is modeled within the evaluator.
   public var percentEscapedString: String {
     @_semantics("string.escapePercent.get")
     @_effects(readonly)
@@ -292,6 +293,7 @@ public struct OSLogMessage :
   @inlinable
   @_optimize(none)
   @_semantics("oslog.message.init_interpolation")
+  @_semantics("constant_evaluable")
   public init(stringInterpolation: OSLogInterpolation) {
     self.interpolation = stringInterpolation
   }
@@ -301,6 +303,7 @@ public struct OSLogMessage :
   @inlinable
   @_optimize(none)
   @_semantics("oslog.message.init_stringliteral")
+  @_semantics("constant_evaluable")
   public init(stringLiteral value: String) {
     var s = OSLogInterpolation()
     s.appendLiteral(value)
@@ -332,7 +335,7 @@ internal struct OSLogArguments {
 
   /// This function must be constant evaluable.
   @inlinable
-  @_semantics("oslog.arguments.init_empty")
+  @_semantics("constant_evaluable")
   @_optimize(none)
   internal init() {
     argumentClosures = nil

stdlib/private/OSLog/OSLogStringTypes.swift

@@ -74,7 +74,7 @@ extension OSLogInterpolation {
   /// This function must be constant evaluable and all its arguments
   /// must be known at compile time.
   @inlinable
-  @_semantics("oslog.interpolation.getFormatSpecifier")
+  @_semantics("constant_evaluable")
   @_effects(readonly)
   @_optimize(none)
   internal func getStringFormatSpecifier(_ isPrivate: Bool) -> String {
@@ -102,7 +102,7 @@ extension OSLogArguments {
 @inlinable
 @_optimize(none)
 @_effects(readonly)
-@_semantics("oslog.string.sizeForEncoding")
+@_semantics("constant_evaluable")
 internal func sizeForEncoding() -> Int {
   return Int.bitWidth &>> logBitsPerByte
 }

test/SILOptimizer/OSLogConstantEvaluableTest.swift

@@ -0,0 +1,51 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-silgen -primary-file %s -o %t/OSLogConstantEvaluableTest_silgen.sil
+//
+// Run the (mandatory) passes on which constant evaluator depends, and run the
+// constant evaluator on the SIL produced after the dependent passes are run.
+//
+// RUN: %target-sil-opt -silgen-cleanup -raw-sil-inst-lowering -allocbox-to-stack -mandatory-inlining -constexpr-limit 1024 -test-constant-evaluable-subset %t/OSLogConstantEvaluableTest_silgen.sil > %t/OSLogConstantEvaluableTest.sil 2> %t/error-output
+//
+// RUN: %FileCheck %s < %t/error-output
+//
+// REQUIRES: OS=macosx || OS=ios || OS=tvos || OS=watchos
+
+// 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 OSLogPrototype
+
+// CHECK-LABEL: @init(stringLiteral: String) -> OSLogMessage
+// CHECK-NOT: error:
+@_semantics("test_driver")
+func osLogMessageStringLiteralInitTest() -> OSLogMessage {
+  return "A string literal"
+}
+
+// CHECK-LABEL: @isPrivate(Privacy) -> Bool
+// CHECK-NOT: error:
+// CHECK-LABEL: @getIntegerFormatSpecifier<A where A: FixedWidthInteger>(A.Type, IntFormat, Bool) -> String
+// CHECK-NOT: error:
+// CHECK-LABEL: @sizeForEncoding<A where A: FixedWidthInteger>(A.Type) -> Int
+// CHECK-NOT: error:
+// CHECK-LABEL: @getArgumentHeader(isPrivate: Bool, type: ArgumentType) -> UInt8
+// CHECK-NOT: error:
+// CHECK-LABEL: @getUpdatedPreamble(isPrivate: Bool, isScalar: Bool) -> UInt8
+// 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: @getStringFormatSpecifier(Bool) -> String
+// CHECK-NOT: error:
+// CHECK-LABEL: @sizeForEncoding() -> Int
+// CHECK-NOT: error:
+@_semantics("test_driver")
+func stringValueInterpolationTest() -> OSLogMessage {
+  return "A string value \("xyz")"
+}

test/SILOptimizer/constant_evaluator_test.sil

@@ -279,7 +279,7 @@ bb0:
   %0 = integer_literal $Builtin.Int64, -5
   %2 = function_ref @factorial : $@convention(thin) (Builtin.Int64) -> Builtin.Int64
   %3 = apply %2(%0) : $@convention(thin) (Builtin.Int64) -> Builtin.Int64
-    // CHECK: {{.*}}:[[@LINE-1]]:{{.*}}: note: exceeded instruction limit: 512 when evaluating the expression at compile time
+    // CHECK: {{.*}}:[[@LINE-1]]:{{.*}}: note: exceeded instruction limit: {{.*}} when evaluating the expression at compile time
     // CHECK: {{.*}}: note: limit exceeded here
   return %3 : $Builtin.Int64
 }

test/SILOptimizer/pound_assert_test_recursive.swift

@@ -10,7 +10,7 @@
 // match what we actually want.
 
 // CHECK: error: #assert condition not constant
-// CHECK: note: exceeded instruction limit: 512 when evaluating the expression at compile time
+// CHECK: note: exceeded instruction limit: {{.*}} when evaluating the expression at compile time
 // CHECK: limit exceeded here
 func recursive(a: Int) -> Int {
   return a == 0 ? 0 : recursive(a: a-1)