Merge pull request #816 from kmod/throw_capis3

Use profiling to know when to throw CAPI exceptions

Makefile

@@ -985,6 +985,7 @@ $1: nosearch_$1
 $1: $(TESTS_DIR)/nosearch_$1 ;
 $1: $(TEST_DIR)/cpython/nosearch_$1 ;
 $1: $(TEST_DIR)/testsuite/integration/nosearch_$1 ;
+$1: $(TEST_DIR)/testsuite/extra/nosearch_$1 ;
 $1: $(TEST_DIR)/extra/nosearch_$1 ;
 $1: ./microbenchmarks/nosearch_$1 ;
 $1: ./minibenchmarks/nosearch_$1 ;

src/codegen/ast_interpreter.cpp

@@ -714,7 +714,7 @@ Box* ASTInterpreter::doOSR(AST_Jump* node) {
     sorted_symbol_table[source_info->getInternedStrings().get(FRAME_INFO_PTR_NAME)] = (Box*)&frame_info;
 
     if (found_entry == nullptr) {
-        OSREntryDescriptor* entry = OSREntryDescriptor::create(clfunc, node);
+        OSREntryDescriptor* entry = OSREntryDescriptor::create(clfunc, node, CXX);
 
         for (auto& it : sorted_symbol_table) {
             if (isIsDefinedName(it.first))

src/codegen/irgen/hooks.cpp

@@ -184,7 +184,8 @@ static void compileIR(CompiledFunction* cf, EffortLevel effort) {
 // should only be called after checking to see if the other versions would work.
 // The codegen_lock needs to be held in W mode before calling this function:
 CompiledFunction* compileFunction(CLFunction* f, FunctionSpecialization* spec, EffortLevel effort,
-                                  const OSREntryDescriptor* entry_descriptor) {
+                                  const OSREntryDescriptor* entry_descriptor, bool force_exception_style,
+                                  ExceptionStyle forced_exception_style) {
     UNAVOIDABLE_STAT_TIMER(t0, "us_timer_compileFunction");
     Timer _t("for compileFunction()", 1000);
 
@@ -197,11 +198,17 @@ CompiledFunction* compileFunction(CLFunction* f, FunctionSpecialization* spec, E
 
     ASSERT(f->versions.size() < 20, "%s %ld", name.c_str(), f->versions.size());
 
-    ExceptionStyle exception_style = CXX;
-    if (FORCE_LLVM_CAPI_THROWS)
+    ExceptionStyle exception_style;
+    if (force_exception_style)
+        exception_style = forced_exception_style;
+    else if (FORCE_LLVM_CAPI_THROWS)
         exception_style = CAPI;
-    if (name == "next")
+    else if (name == "next")
         exception_style = CAPI;
+    else if (f->propagated_cxx_exceptions >= 100)
+        exception_style = CAPI;
+    else
+        exception_style = CXX;
 
     if (VERBOSITY("irgen") >= 1) {
         std::string s;
@@ -764,9 +771,8 @@ static CompiledFunction* _doReopt(CompiledFunction* cf, EffortLevel new_effort)
         if (versions[i] == cf) {
             versions.erase(versions.begin() + i);
 
-            CompiledFunction* new_cf
-                = compileFunction(clfunc, cf->spec, new_effort,
-                                  NULL); // this pushes the new CompiledVersion to the back of the version list
+            // this pushes the new CompiledVersion to the back of the version list
+            CompiledFunction* new_cf = compileFunction(clfunc, cf->spec, new_effort, NULL, true, cf->exception_style);
 
             cf->dependent_callsites.invalidateAll();
 
@@ -797,7 +803,8 @@ CompiledFunction* compilePartialFuncInternal(OSRExit* exit) {
     CompiledFunction*& new_cf = clfunc->osr_versions[exit->entry];
     if (new_cf == NULL) {
         EffortLevel new_effort = EffortLevel::MAXIMAL;
-        CompiledFunction* compiled = compileFunction(clfunc, NULL, new_effort, exit->entry);
+        CompiledFunction* compiled
+            = compileFunction(clfunc, NULL, new_effort, exit->entry, true, exit->entry->exception_style);
         assert(compiled == new_cf);
 
         stat_osr_compiles.log();
@@ -807,7 +814,9 @@ CompiledFunction* compilePartialFuncInternal(OSRExit* exit) {
 }
 
 void* compilePartialFunc(OSRExit* exit) {
-    return compilePartialFuncInternal(exit)->code;
+    CompiledFunction* new_cf = compilePartialFuncInternal(exit);
+    assert(new_cf->exception_style == exit->entry->exception_style);
+    return new_cf->code;
 }
 
 

src/codegen/irgen/irgenerator.cpp

@@ -2140,7 +2140,7 @@ private:
 
         // Emitting the actual OSR:
         emitter.getBuilder()->SetInsertPoint(onramp);
-        OSREntryDescriptor* entry = OSREntryDescriptor::create(irstate->getCL(), osr_key);
+        OSREntryDescriptor* entry = OSREntryDescriptor::create(irstate->getCL(), osr_key, irstate->getExceptionStyle());
         OSRExit* exit = new OSRExit(entry);
         llvm::Value* partial_func = emitter.getBuilder()->CreateCall(g.funcs.compilePartialFunc,
                                                                      embedRelocatablePtr(exit, g.i8->getPointerTo()));
@@ -2296,17 +2296,26 @@ private:
         // It looks like ommitting the second and third arguments are equivalent to passing None,
         // but ommitting the first argument is *not* the same as passing None.
 
-        ExceptionStyle target_exception_style = CXX;
-        if (unw_info.preferredExceptionStyle() == CAPI && (node->arg0 && !node->arg2))
+        ExceptionStyle target_exception_style;
+
+        if (unw_info.hasHandler())
             target_exception_style = CAPI;
+        else
+            target_exception_style = irstate->getExceptionStyle();
 
         if (node->arg0 == NULL) {
             assert(!node->arg1);
             assert(!node->arg2);
 
-            assert(target_exception_style == CXX);
-            emitter.createCall(unw_info, g.funcs.raise0, std::vector<llvm::Value*>());
-            emitter.getBuilder()->CreateUnreachable();
+            if (target_exception_style == CAPI) {
+                emitter.createCall(unw_info, g.funcs.raise0_capi, std::vector<llvm::Value*>(), CAPI);
+                emitter.checkAndPropagateCapiException(unw_info, getNullPtr(g.llvm_value_type_ptr),
+                                                       getNullPtr(g.llvm_value_type_ptr));
+                emitter.getBuilder()->CreateUnreachable();
+            } else {
+                emitter.createCall(unw_info, g.funcs.raise0, std::vector<llvm::Value*>());
+                emitter.getBuilder()->CreateUnreachable();
+            }
 
             endBlock(DEAD);
             return;

src/codegen/osrentry.h

@@ -31,16 +31,20 @@ struct StackMap;
 
 class OSREntryDescriptor {
 private:
-    OSREntryDescriptor(CLFunction* clfunc, AST_Jump* backedge) : clfunc(clfunc), backedge(backedge) { assert(clfunc); }
+    OSREntryDescriptor(CLFunction* clfunc, AST_Jump* backedge, ExceptionStyle exception_style)
+        : clfunc(clfunc), backedge(backedge), exception_style(exception_style) {
+        assert(clfunc);
+    }
 
 public:
     CLFunction* clfunc;
     AST_Jump* const backedge;
+    ExceptionStyle exception_style;
     typedef std::map<InternedString, ConcreteCompilerType*> ArgMap;
     ArgMap args;
 
-    static OSREntryDescriptor* create(CLFunction* clfunc, AST_Jump* backedge) {
-        return new OSREntryDescriptor(clfunc, backedge);
+    static OSREntryDescriptor* create(CLFunction* clfunc, AST_Jump* backedge, ExceptionStyle exception_style) {
+        return new OSREntryDescriptor(clfunc, backedge, exception_style);
     }
 };
 

src/codegen/runtime_hooks.cpp

@@ -307,6 +307,7 @@ void initGlobalFuncs(GlobalState& g) {
 
     g.funcs.__cxa_end_catch = addFunc((void*)__cxa_end_catch, g.void_);
     GET(raise0);
+    GET(raise0_capi);
     GET(raise3);
     GET(raise3_capi);
     GET(PyErr_Fetch);

src/codegen/runtime_hooks.h

@@ -50,7 +50,7 @@ struct GlobalFuncs {
     llvm::Value* boxedLocalsSet, *boxedLocalsGet, *boxedLocalsDel;
 
     llvm::Value* __cxa_end_catch;
-    llvm::Value* raise0, *raise3, *raise3_capi;
+    llvm::Value* raise0, *raise0_capi, *raise3, *raise3_capi;
     llvm::Value* PyErr_Fetch, *PyErr_NormalizeException, *PyErr_Restore, *caughtCapiException, *reraiseCapiExcAsCxx;
     llvm::Value* deopt;
 

src/codegen/unwinding.cpp

@@ -556,6 +556,8 @@ public:
         PythonFrameIteratorImpl frame_iter;
         bool found_frame = pystack_extractor.handleCFrame(cursor, &frame_iter);
         if (found_frame) {
+            frame_iter.getCL()->propagated_cxx_exceptions++;
+
             if (exceptionAtLineCheck()) {
                 // TODO: shouldn't fetch this multiple times?
                 frame_iter.getCurrentStatement()->cxx_exception_count++;

src/core/types.h

@@ -376,6 +376,8 @@ public:
     // Please use codeForFunction() to access this:
     BoxedCode* code_obj;
 
+    int propagated_cxx_exceptions = 0;
+
     // For use by the interpreter/baseline jit:
     int times_interpreted;
     std::vector<std::unique_ptr<JitCodeBlock>> code_blocks;
@@ -438,7 +440,8 @@ CLFunction* unboxRTFunction(Box*);
 // Compiles a new version of the function with the given signature and adds it to the list;
 // should only be called after checking to see if the other versions would work.
 CompiledFunction* compileFunction(CLFunction* f, FunctionSpecialization* spec, EffortLevel effort,
-                                  const OSREntryDescriptor* entry);
+                                  const OSREntryDescriptor* entry, bool force_exception_style = false,
+                                  ExceptionStyle forced_exception_style = CXX);
 EffortLevel initialEffort();
 
 typedef bool i1;

src/runtime/exceptions.cpp

@@ -68,19 +68,6 @@ void raiseSyntaxErrorHelper(llvm::StringRef file, llvm::StringRef func, AST* nod
     raiseSyntaxError(buf, node_at->lineno, node_at->col_offset, file, "");
 }
 
-extern "C" void raise0() {
-    ExcInfo* exc_info = getFrameExcInfo();
-    assert(exc_info->type);
-
-    // TODO need to clean up when we call normalize, do_raise, etc
-    if (exc_info->type == None)
-        raiseExcHelper(TypeError, "exceptions must be old-style classes or derived from BaseException, not NoneType");
-
-    startReraise();
-    assert(!PyErr_Occurred());
-    throw * exc_info;
-}
-
 void ExcInfo::printExcAndTraceback() const {
     PyErr_Display(type, value, traceback);
 }
@@ -148,6 +135,35 @@ ExcInfo excInfoForRaise(Box* type, Box* value, Box* tb) {
     return ExcInfo(type, value, tb);
 }
 
+extern "C" void raise0() {
+    ExcInfo* exc_info = getFrameExcInfo();
+    assert(exc_info->type);
+
+    // TODO need to clean up when we call normalize, do_raise, etc
+    if (exc_info->type == None)
+        raiseExcHelper(TypeError, "exceptions must be old-style classes or derived from BaseException, not NoneType");
+
+    startReraise();
+    assert(!PyErr_Occurred());
+    throw * exc_info;
+}
+
+extern "C" void raise0_capi() noexcept {
+    ExcInfo exc_info = *getFrameExcInfo();
+    assert(exc_info.type);
+
+    // TODO need to clean up when we call normalize, do_raise, etc
+    if (exc_info.type == None) {
+        exc_info.type = TypeError;
+        exc_info.value = boxString("exceptions must be old-style classes or derived from BaseException, not NoneType");
+        exc_info.traceback = None;
+        PyErr_NormalizeException(&exc_info.type, &exc_info.value, &exc_info.traceback);
+    }
+
+    startReraise();
+    PyErr_Restore(exc_info.type, exc_info.value, exc_info.traceback);
+}
+
 extern "C" void raise3(Box* arg0, Box* arg1, Box* arg2) {
     bool reraise = arg2 != NULL && arg2 != None;
     auto exc_info = excInfoForRaise(arg0, arg1, arg2);
@@ -169,7 +185,9 @@ extern "C" void raise3_capi(Box* arg0, Box* arg1, Box* arg2) noexcept {
         exc_info = e;
     }
 
-    assert(!reraise); // would get thrown away
+    if (reraise)
+        startReraise();
+
     PyErr_Restore(exc_info.type, exc_info.value, exc_info.traceback);
 }
 

src/runtime/inline/link_forcer.cpp

@@ -125,6 +125,7 @@ void force() {
     FORCE(callattrCapi);
 
     FORCE(raise0);
+    FORCE(raise0_capi);
     FORCE(raise3);
     FORCE(raise3_capi);
     FORCE(PyErr_Fetch);

src/runtime/objmodel.h

@@ -34,6 +34,7 @@ class BoxedTuple;
 // user-level raise functions that implement python-level semantics
 ExcInfo excInfoForRaise(Box*, Box*, Box*);
 extern "C" void raise0() __attribute__((__noreturn__));
+extern "C" void raise0_capi() noexcept;
 extern "C" void raise3(Box*, Box*, Box*) __attribute__((__noreturn__));
 extern "C" void raise3_capi(Box*, Box*, Box*) noexcept;
 void raiseExc(Box* exc_obj) __attribute__((__noreturn__));

test/unittests/analysis.cpp

@@ -92,7 +92,7 @@ void doOsrTest(bool is_osr, bool i_maybe_undefined) {
     std::unique_ptr<PhiAnalysis> phis;
 
     if (is_osr) {
-        OSREntryDescriptor* entry_descriptor = OSREntryDescriptor::create(clfunc, backedge);
+        OSREntryDescriptor* entry_descriptor = OSREntryDescriptor::create(clfunc, backedge, CXX);
         entry_descriptor->args[i_str] = NULL;
         if (i_maybe_undefined)
             entry_descriptor->args[idi_str] = NULL;