From 0cea6ff633658d77d686766290e40adfd6657e89 Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Fri, 24 Jan 2020 10:23:06 -0800 Subject: [PATCH] Turn off speculative devirtualization by default. This is a cherrypick of #29359 Explanation: Disable speculative devirtualization optimization. The optimization did not give significant performance wins. Disabling it improves code size. Risk: Low. No significant performance hit is expected. Issue: rdar://problem/58778959 Testing: Performance tests and regression tests Reviewer: Erik Eckstein --- include/swift/AST/SILOptions.h | 4 ++++ include/swift/Option/FrontendOptions.td | 3 +++ lib/Frontend/CompilerInvocation.cpp | 1 + lib/SILOptimizer/PassManager/PassPipeline.cpp | 4 +++- .../Transforms/SpeculativeDevirtualizer.cpp | 1 + test/SILOptimizer/devirt_base_class.swift | 2 +- .../devirt_covariant_return.swift | 2 +- test/SILOptimizer/devirt_default_case.swift | 4 ++-- .../devirt_protocol_method_invocations.swift | 2 +- test/SILOptimizer/devirt_speculate.swift | 2 +- .../devirt_speculative_init.swift | 2 +- .../devirt_speculative_nested.swift | 2 +- .../SILOptimizer/devirt_unbound_generic.swift | 2 +- .../SILOptimizer/devirt_value_metatypes.swift | 2 +- test/SILOptimizer/inline_cache_and_arc.swift | 3 +-- test/SILOptimizer/opt_mode.swift | 22 +++++++++---------- tools/sil-opt/SILOpt.cpp | 6 +++++ 17 files changed, 40 insertions(+), 24 deletions(-) diff --git a/include/swift/AST/SILOptions.h b/include/swift/AST/SILOptions.h index 75e794074f8af..ce3d760b58c86 100644 --- a/include/swift/AST/SILOptions.h +++ b/include/swift/AST/SILOptions.h @@ -57,6 +57,10 @@ class SILOptions { /// purposes. bool EnableOSSAOptimizations = true; + /// Controls whether to turn on speculative devirtualization. + /// It is turned off by default. + bool EnableSpeculativeDevirtualization = false; + /// Should we run any SIL performance optimizations /// /// Useful when you want to enable -O LLVM opts but not -O SIL opts. diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index 7b1124d0f4348..d371661a1dbbf 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -278,6 +278,9 @@ def disable_ossa_opts : Flag<["-"], "disable-ossa-opts">, def disable_sil_partial_apply : Flag<["-"], "disable-sil-partial-apply">, HelpText<"Disable use of partial_apply in SIL generation">; +def enable_spec_devirt : Flag<["-"], "enable-spec-devirt">, + HelpText<"Enable speculative devirtualization pass.">; + def enable_ownership_stripping_after_serialization : Flag<["-"], "enable-ownership-stripping-after-serialization">, HelpText<"Strip ownership after serialization">; diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index df55e145069f5..acf0e7ccad803 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -877,6 +877,7 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args, Opts.EnableARCOptimizations &= !Args.hasArg(OPT_disable_arc_opts); Opts.EnableOSSAOptimizations &= !Args.hasArg(OPT_disable_ossa_opts); + Opts.EnableSpeculativeDevirtualization |= Args.hasArg(OPT_enable_spec_devirt); Opts.DisableSILPerfOptimizations |= Args.hasArg(OPT_disable_sil_perf_optzns); Opts.CrossModuleOptimization |= Args.hasArg(OPT_CrossModuleOptimization); Opts.VerifyAll |= Args.hasArg(OPT_sil_verify_all); diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp index 46626b254534a..c4d2766c047a8 100644 --- a/lib/SILOptimizer/PassManager/PassPipeline.cpp +++ b/lib/SILOptimizer/PassManager/PassPipeline.cpp @@ -489,7 +489,9 @@ static void addClosureSpecializePassPipeline(SILPassPipelinePlan &P) { P.addStackPromotion(); // Speculate virtual call targets. - P.addSpeculativeDevirtualization(); + if (P.getOptions().EnableSpeculativeDevirtualization) { + P.addSpeculativeDevirtualization(); + } // There should be at least one SILCombine+SimplifyCFG between the // ClosureSpecializer, etc. and the last inliner. Cleaning up after these diff --git a/lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp b/lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp index 5c2fb108633a1..2ca01e247fd21 100644 --- a/lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp +++ b/lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp @@ -596,6 +596,7 @@ namespace { void run() override { auto &CurFn = *getFunction(); + // Don't perform speculative devirtualization at -Os. if (CurFn.optimizeForSize()) return; diff --git a/test/SILOptimizer/devirt_base_class.swift b/test/SILOptimizer/devirt_base_class.swift index cf82d7b893c82..76239f4259366 100644 --- a/test/SILOptimizer/devirt_base_class.swift +++ b/test/SILOptimizer/devirt_base_class.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -O -emit-sil %s | %FileCheck %s +// RUN: %target-swift-frontend -enable-spec-devirt -O -emit-sil %s | %FileCheck %s public class Base1 { @inline(never) func f() -> Int { return 0 } } diff --git a/test/SILOptimizer/devirt_covariant_return.swift b/test/SILOptimizer/devirt_covariant_return.swift index dc12740d9343f..0f63101cc62cd 100644 --- a/test/SILOptimizer/devirt_covariant_return.swift +++ b/test/SILOptimizer/devirt_covariant_return.swift @@ -1,5 +1,5 @@ -// RUN: %target-swift-frontend -module-name devirt_covariant_return -Xllvm -sil-full-demangle -O -Xllvm -disable-sil-cm-rr-cm=0 -Xllvm -sil-inline-generics=false -primary-file %s -emit-sil -sil-inline-threshold 1000 -Xllvm -sil-disable-pass=ObjectOutliner -sil-verify-all | %FileCheck %s +// RUN: %target-swift-frontend -module-name devirt_covariant_return -Xllvm -sil-full-demangle -enable-spec-devirt -O -Xllvm -disable-sil-cm-rr-cm=0 -Xllvm -sil-inline-generics=false -primary-file %s -emit-sil -sil-inline-threshold 1000 -Xllvm -sil-disable-pass=ObjectOutliner -sil-verify-all | %FileCheck %s // Make sure that we can dig all the way through the class hierarchy and // protocol conformances with covariant return types correctly. The verifier diff --git a/test/SILOptimizer/devirt_default_case.swift b/test/SILOptimizer/devirt_default_case.swift index d3598e59df367..6d485265edf67 100644 --- a/test/SILOptimizer/devirt_default_case.swift +++ b/test/SILOptimizer/devirt_default_case.swift @@ -1,6 +1,6 @@ -// RUN: %target-swift-frontend -O -module-name devirt_default_case -emit-sil %s | %FileCheck -check-prefix=CHECK -check-prefix=CHECK-NORMAL %s -// RUN: %target-swift-frontend -O -module-name devirt_default_case -emit-sil -enable-testing %s | %FileCheck -check-prefix=CHECK -check-prefix=CHECK-TESTABLE %s +// RUN: %target-swift-frontend -enable-spec-devirt -O -module-name devirt_default_case -emit-sil %s | %FileCheck -check-prefix=CHECK -check-prefix=CHECK-NORMAL %s +// RUN: %target-swift-frontend -enable-spec-devirt -O -module-name devirt_default_case -emit-sil -enable-testing %s | %FileCheck -check-prefix=CHECK -check-prefix=CHECK-TESTABLE %s @_silgen_name("action") func action(_ n:Int) -> () diff --git a/test/SILOptimizer/devirt_protocol_method_invocations.swift b/test/SILOptimizer/devirt_protocol_method_invocations.swift index 1cddbd37c749c..3741185505380 100644 --- a/test/SILOptimizer/devirt_protocol_method_invocations.swift +++ b/test/SILOptimizer/devirt_protocol_method_invocations.swift @@ -1,5 +1,5 @@ -// RUN: %target-swift-frontend -module-name devirt_protocol_method_invocations -O -Xllvm -sil-disable-pass=ExistentialSpecializer -emit-sil %s | %FileCheck %s +// RUN: %target-swift-frontend -module-name devirt_protocol_method_invocations -enable-spec-devirt -O -Xllvm -sil-disable-pass=ExistentialSpecializer -emit-sil %s | %FileCheck %s protocol PPP { func f() diff --git a/test/SILOptimizer/devirt_speculate.swift b/test/SILOptimizer/devirt_speculate.swift index e45f573d1147a..7167716153be1 100644 --- a/test/SILOptimizer/devirt_speculate.swift +++ b/test/SILOptimizer/devirt_speculate.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend %/s -parse-as-library -O -emit-sil -save-optimization-record-path %t.opt.yaml | %FileCheck %s +// RUN: %target-swift-frontend %/s -parse-as-library -enable-spec-devirt -O -emit-sil -save-optimization-record-path %t.opt.yaml | %FileCheck %s // RUN: %FileCheck -check-prefix=YAML -input-file=%t.opt.yaml %s // RUN: %target-swift-frontend %/s -parse-as-library -Osize -emit-sil | %FileCheck %s --check-prefix=OSIZE // diff --git a/test/SILOptimizer/devirt_speculative_init.swift b/test/SILOptimizer/devirt_speculative_init.swift index 28ce888723bfe..7f126ca1cea59 100644 --- a/test/SILOptimizer/devirt_speculative_init.swift +++ b/test/SILOptimizer/devirt_speculative_init.swift @@ -1,5 +1,5 @@ -// RUN: %target-swift-frontend %s -parse-as-library -O -emit-sil | %FileCheck %s +// RUN: %target-swift-frontend %s -parse-as-library -enable-spec-devirt -O -emit-sil | %FileCheck %s // RUN: %target-swift-frontend %s -parse-as-library -Osize -emit-sil // // Test speculative devirtualization. diff --git a/test/SILOptimizer/devirt_speculative_nested.swift b/test/SILOptimizer/devirt_speculative_nested.swift index c6c5fec6ad6fa..5dc8d089c6ecf 100644 --- a/test/SILOptimizer/devirt_speculative_nested.swift +++ b/test/SILOptimizer/devirt_speculative_nested.swift @@ -1,5 +1,5 @@ -// RUN: %target-swift-frontend -module-name devirt_speculative_nested %s -parse-as-library -O -emit-sil | %FileCheck %s +// RUN: %target-swift-frontend -module-name devirt_speculative_nested %s -parse-as-library -enable-spec-devirt -O -emit-sil | %FileCheck %s // RUN: %target-swift-frontend -module-name devirt_speculative_nested %s -parse-as-library -Osize -emit-sil // // Test speculative devirtualization. diff --git a/test/SILOptimizer/devirt_unbound_generic.swift b/test/SILOptimizer/devirt_unbound_generic.swift index e36a0f440cffb..67427ef9223ca 100644 --- a/test/SILOptimizer/devirt_unbound_generic.swift +++ b/test/SILOptimizer/devirt_unbound_generic.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -Xllvm -sil-inline-generics -emit-sorted-sil -emit-sil -O %s | %FileCheck %s +// RUN: %target-swift-frontend -Xllvm -sil-inline-generics -emit-sorted-sil -emit-sil -enable-spec-devirt -O %s | %FileCheck %s // We used to crash on this when trying to devirtualize t.boo(a, 1), // because it is an "apply" with replacement types that contain diff --git a/test/SILOptimizer/devirt_value_metatypes.swift b/test/SILOptimizer/devirt_value_metatypes.swift index 215ebfe9fc4d8..23dd0e226376b 100644 --- a/test/SILOptimizer/devirt_value_metatypes.swift +++ b/test/SILOptimizer/devirt_value_metatypes.swift @@ -1,5 +1,5 @@ -// RUN: %target-swift-frontend -module-name devirt_value_metatypes -emit-sil -O %s | %FileCheck %s +// RUN: %target-swift-frontend -module-name devirt_value_metatypes -emit-sil -enable-spec-devirt -O %s | %FileCheck %s open class A { @inline(never) diff --git a/test/SILOptimizer/inline_cache_and_arc.swift b/test/SILOptimizer/inline_cache_and_arc.swift index 0601f30466049..f251ee07fd56f 100644 --- a/test/SILOptimizer/inline_cache_and_arc.swift +++ b/test/SILOptimizer/inline_cache_and_arc.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -parse-as-library -O -emit-sil %s | %FileCheck %s +// RUN: %target-swift-frontend -parse-as-library -enable-spec-devirt -O -emit-sil %s | %FileCheck %s // REQUIRES: swift_stdlib_no_asserts,optimized_stdlib,CPU=x86_64 // Test inline cache with a global class. Make sure the retain|release pair @@ -19,7 +19,6 @@ public func testit() { // CHECK-LABEL: sil @{{.*}}testityyF // CHECK: bb0: // CHECK-NOT: {{.*(retain|release).*}} -// CHECK: checked_cast_br // CHECK: bb1{{.*}}: // CHECK-NOT: {{.*(retain|release).*}} // CHECK: return diff --git a/test/SILOptimizer/opt_mode.swift b/test/SILOptimizer/opt_mode.swift index d2257d7c1d0ad..cfe34c25aea42 100644 --- a/test/SILOptimizer/opt_mode.swift +++ b/test/SILOptimizer/opt_mode.swift @@ -15,16 +15,6 @@ class B : A { func donothing(_ x: Int) -> Int { return x } -// CHECK-LABEL: sil {{.*}} [Ospeed] @{{.*}}test_ospeed -// CHECK: checked_cast_br -// CHECK: checked_cast_br -// CHECK: } -// CHECK-IR: define hidden {{.*}}test_ospeed{{.*}} [[NOSIZE_ATTR:#[0-9]+]] -@_optimize(speed) -func test_ospeed(_ a: A) -> Int { - return donothing(a.foo(27)) -} - // CHECK-LABEL: sil {{.*}} [Osize] @{{.*}}test_osize // CHECK: [[M:%[0-9]+]] = class_method // CHECK: [[A:%[0-9]+]] = apply [[M]] @@ -46,6 +36,16 @@ func test_onone(_ a: A) -> Int { return donothing(a.foo(27)) } +// CHECK-LABEL: sil {{.*}} [Ospeed] @{{.*}}test_ospeed +// CHECK: [[M:%[0-9]+]] = class_method +// CHECK: [[A:%[0-9]+]] = apply [[M]] +// CHECK: return [[A]] +// CHECK-IR: define hidden {{.*}}test_ospeed{{.*}} [[NOSIZE_ATTR:#[0-9]+]] +@_optimize(speed) +func test_ospeed(_ a: A) -> Int { + return donothing(a.foo(27)) +} + -// CHECK-IR: attributes [[NOSIZE_ATTR]] = { " // CHECK-IR: attributes [[SIZE_ATTR]] = { minsize " +// CHECK-IR: attributes [[NOSIZE_ATTR]] = { " diff --git a/tools/sil-opt/SILOpt.cpp b/tools/sil-opt/SILOpt.cpp index 91e00140cd426..886a6fbe64770 100644 --- a/tools/sil-opt/SILOpt.cpp +++ b/tools/sil-opt/SILOpt.cpp @@ -102,6 +102,10 @@ static llvm::cl::opt VerifyExclusivity("enable-verify-exclusivity", llvm::cl::desc("Verify the access markers used to enforce exclusivity.")); +static llvm::cl::opt +EnableSpeculativeDevirtualization("enable-spec-devirt", + llvm::cl::desc("Enable Speculative Devirtualization pass.")); + namespace { enum EnforceExclusivityMode { Unchecked, // static only @@ -376,6 +380,8 @@ int main(int argc, char **argv) { } } + SILOpts.EnableSpeculativeDevirtualization = EnableSpeculativeDevirtualization; + serialization::ExtendedValidationInfo extendedInfo; llvm::ErrorOr> FileBufOrErr = Invocation.setUpInputForSILTool(InputFilename, ModuleName,