https://github.com/jyu2-git updated https://github.com/llvm/llvm-project/pull/71488
>From 3313aca0622da3882a9e5bf304b89f28fecce7fe Mon Sep 17 00:00:00 2001 From: Jennifer Yu <jennifer...@intel.com> Date: Mon, 6 Nov 2023 20:51:39 -0800 Subject: [PATCH 1/2] [SEH] Fix assertin when return scalar value from __try block. Current compler assert with `!SI->isAtomic() && !SI->isVolatile()' failed This due to following rule: First, no exception can move in or out of _try region., i.e., no "potential faulty instruction can be moved across _try boundary. Second, the order of exceptions for instructions 'directly' under a _try must be preserved (not applied to those in callees). Finally, global states (local/global/heap variables) that can be read outside of _try region must be updated in memory (not just in register) before the subsequent exception occurs. All memory instructions inside a _try are considered as 'volatile' to assure 2nd and 3rd rules for C-code above. This is a little sub-optimized. But it's acceptable as the amount of code directly under _try is very small. However during findDominatingStoreToReturnValue: those are not allowed. To fix just skip the assertion when current function has seh try. --- clang/lib/CodeGen/CGCall.cpp | 3 +++ .../CodeGen/windows-seh-EHa-TryInFinally.cpp | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index e7951b3a3f4d855..bc367b89992bbd9 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -3507,6 +3507,9 @@ static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) { return nullptr; // These aren't actually possible for non-coerced returns, and we // only care about non-coerced returns on this code path. + // All memory instructions inside __try block are volatile. + if (CGF.currentFunctionUsesSEHTry() && SI->isVolatile()) + return SI; assert(!SI->isAtomic() && !SI->isVolatile()); return SI; }; diff --git a/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp b/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp index fab567e763df443..ce2a9528e1908a9 100644 --- a/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp +++ b/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp @@ -67,3 +67,26 @@ void foo() } } } + +// CHECK-LABEL:@"?bar@@YAHXZ"() +// CHECK: invoke.cont: +// CHECK: invoke void @llvm.seh.try.begin() +// CHECK: invoke.cont1: +// CHECK: store volatile i32 1, ptr %cleanup.dest.slot +// CHECK: invoke void @llvm.seh.try.end() +// CHECK: invoke.cont2: +// CHECK: call void @"?fin$0@0@bar@@" +// CHECK: %cleanup.dest3 = load i32, ptr %cleanup.dest.slot +// CHECK: return: +// CHECK: ret i32 11 +int bar() +{ + int x; + __try { + return 11; + } __finally { + if (_abnormal_termination()) { + x = 9; + } + } +} >From 4caaaef0ada4d22b91c38704da90a2c1debe8e55 Mon Sep 17 00:00:00 2001 From: Jennifer Yu <jennifer...@intel.com> Date: Tue, 7 Nov 2023 15:24:47 -0800 Subject: [PATCH 2/2] Thanks @rnk for the review. This address his comment. --- clang/lib/CodeGen/CGCall.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index bc367b89992bbd9..8fd876825b7ea08 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -3508,9 +3508,8 @@ static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) { // These aren't actually possible for non-coerced returns, and we // only care about non-coerced returns on this code path. // All memory instructions inside __try block are volatile. - if (CGF.currentFunctionUsesSEHTry() && SI->isVolatile()) - return SI; - assert(!SI->isAtomic() && !SI->isVolatile()); + assert(!SI->isAtomic() && + (!SI->isVolatile() || CGF.currentFunctionUsesSEHTry())); return SI; }; // If there are multiple uses of the return-value slot, just check _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits