Page MenuHomePhabricator

tentzen (Ten Tzen)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 10 2020, 4:29 PM (33 w, 5 d)

Recent Activity

Tue, Nov 24

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Do we need to consider FP exceptions in _try block?

Yes, FP exception is handled as long as FP exceptions are not disabled (Ex via _controlfp() runtime) and FP exception code is filtered & handled via ___except() statement (Ex, ___except(GetExceptionCode()==EXCEPTION_FLT_INEXACT_RESULT)).

I see. If this is the case, you may need to assign FPE_Strict to _try block to preserve FP instructions' order.

Tue, Nov 24, 6:53 PM · Restricted Project, Restricted Project

Thu, Nov 19

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Does it really work for x86_64 architecture only? I have tried using this with x86 build and produced binaries looks like this should work on this platform as well, but I have not tried running produced executables to see the results yet tbh.

Thu, Nov 19, 10:52 PM · Restricted Project, Restricted Project
tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Do we need to consider FP exceptions in _try block?

Thu, Nov 19, 10:17 PM · Restricted Project, Restricted Project
tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Per Joseph's feedback, further clarify the semantic of llvm.seh.try.begin and llvm.seh.try.end.

Thu, Nov 19, 12:54 PM · Restricted Project, Restricted Project
tentzen added inline comments to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.
Thu, Nov 19, 12:03 PM · Restricted Project, Restricted Project
tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Fixed the clang fault when built without '-feh-asynch' option.
EHStack's CGF field must be initialized even without -feh-asynch.

Thu, Nov 19, 10:37 AM · Restricted Project, Restricted Project

Wed, Nov 18

tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

per review feedback from John McCall and others, this update includes:

Wed, Nov 18, 7:11 PM · Restricted Project, Restricted Project

Sep 28 2020

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

OK, great.
Summarize our discussion above, I will follow up with:

Sep 28 2020, 1:46 PM · Restricted Project, Restricted Project

Sep 24 2020

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Okay. I think we're on the same page about representation now. If you can come up with a good replacement for "eha" in the intrinsic names, I think this is pretty much ready to go.

Sep 24 2020, 3:56 PM · Restricted Project, Restricted Project
tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Reordering is NOT okay for instructions 'directly' within a _try. 'directly' here means it's the code from source code originally, i.e., inlined code or the code in callees is not included. Disabling inline in general is good, but not necessary.
As said above, SEH semantic only applies to code "directly" under SEH scope for few obvious reasons (Ex, indirect callees or callees not in the same module wont be visible). if faulty instructions are allowed to be reordered inside a callee, those instructions are allowed to do so even after they are inlined into _try.

I feel almost certain that this is wrong. Let me try to explain. It would be helpful to have a more precise statement of semantics than I can find online, but we can start from the basics.

"X and Y can be reordered" is how optimizer folks talk, but it's not generally how semantics are specified. As a general rule (ignoring sequence points and concurrency for simplicity), C and C++ require operations to be executed in a particular order on the abstract machine, and there are no rules explicitly sanctioning reordering. Instead, the "as if" rule gives the implementation broad leeway to do things differently from how they're specified on the abstract machine, as long as the difference cannot be observed by a valid program. Faults generally correspond to conditions that have undefined behavior under the language, and so the implementation may assume they do not happen in a valid program, and so the consequences of them happening cannot be observed by a valid program, and so faults can be ignored when deciding whether to reorder operations, which allows a lot of reordering.

_try should be understood as fully defining the behavior of faulting operations written within its scope so that they have the defined semantics of resuming execution in the _except clause. Because faults have the defined behavior of ending execution of the _try block, whether the fault occurred is observable, so the "as if" leeway stops applying. Thus, other operations with side-effects cannot be reordered with potentially faulty operations written within the _try, no more than they could be reordered with a break statement. That applies whether those other operations are written within the _try or not; it's just that potentially-faulty operations written within the _try must always be treated as having side-effects.

-EHa more generally should be understand as partially defining the behavior of faulting operations even if they are not written in a _try: if the operation faults, the behavior is defined so that scopes are unwound and control resumes at an _except clause if one is dynamically active, but this may be observed at an indeterminate point. It is hard to describe these semantics formally, but the intended rules for the implementation are pretty clear: potentially faulty operations outside of a _try can be reordered around each other or even moved to different scopes, as per normal optimization rules, but whenever those operations are executed, if they fault they must trigger an unwind and cause execution to resume at an _except clause if one is active.

So I think you cannot allow operations inlined from a call made within a _try to be reordered with operations written within the _try, or to happen outside the _try. Since LLVM doesn't promise not to reorder volatile and non-volatile stores to provably non-aliased locations, you cannot safely inline non-volatile stores within a _try. You can either disable inlining or mark the calls in some way that will cause the inliner to make any inlined stores volatile (and whatever else is necessary to ensure they will not be reordered).

Oh, yes you are right. I was talking about reordering between inlined faulty instructions is allowed. Reordering inlined instruction with any 'direct' volatile instruction should be prohibited. I overlooked LLVM doesn't promise not to reorder volatile and non-volatile. I will simply disable inlining into a _try scope. thanks!

Sep 24 2020, 12:40 AM · Restricted Project, Restricted Project

Sep 23 2020

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

There is absolutely NO explicit edge added in this design.

I didn't say there was. I said that there was an *implicit* edge: from the memory access to the handler. It's precisely because that edge is implicit that it's problematic for analysis and optimization.

Oh, sorry I misread it.
For HW exceptions the 'implicit' edge is unavoidable unless an explicit approach (like previous iload/istore) is employed. iload approach was extensively discussed and evaluated when it's proposed. As far as I know, the concerns were that it could create many Invokes, complicate flow graph and possibly result in negative performance impact for downstream optimization and code generation. Making all optimizations be aware of the new semantic is also substantial.

Hmm. I suppose it's not that different from the general problem with setjmp/longjmp. I think you'll still have representation problems here, but I'll address them below; I concede that in principle marking regions with an intrinsic that instructions can't necessary be moved over is workable, and so you don't need to turn every faulting instruction into an invoke. You may need to mark the initial seh.try.begin with something like returns_twice or otherwise communicate that there's non-obvious control flow within that function.

OK, good to know this. thank you!
Other than blocking some opts, does this returns_twice attribute have some other implications?

Sep 23 2020, 5:53 PM · Restricted Project, Restricted Project

Sep 22 2020

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Thank you for prompt reply again.

[rjmccall] And I agree with him that the potential benefits are substantial. I'm just saying that one barely-trafficked RFC thread is not evidence of a community consensus.

OK, thanks. it's good to know you are also supportive in adding this feature.

[rjmccall] As I understand it, you are creating implicit edges within the function to the active SEH handler (potentially part of the same function) from arbitrary places within covered basic blocks, turning basic blocks from a single-entry single-(internal-)exit representation to a single-entry multiple-exit representation. That is a huge change. How do you expect LLVM transformations to preserve or merge this information across blocks? How do you expect transformations to not move code between blocks in problematic ways, or reorder code within blocks in ways that will suddenly be visible to the optimizer? These are the standard objections to supporting features like this, that they have basically unspecified behavior and appear to work by good intentions and happy thoughts.

There is absolutely NO explicit edge added in this design.

I didn't say there was. I said that there was an *implicit* edge: from the memory access to the handler. It's precisely because that edge is implicit that it's problematic for analysis and optimization.

Oh, sorry I misread it.
For HW exceptions the 'implicit' edge is unavoidable unless an explicit approach (like previous iload/istore) is employed. iload approach was extensively discussed and evaluated when it's proposed. As far as I know, the concerns were that it could create many Invokes, complicate flow graph and possibly result in negative performance impact for downstream optimization and code generation. Making all optimizations be aware of the new semantic is also substantial.
So this design applies an IMPLICT approach. I respectfully disagree that it's problematic because as long as volatile attribute is honored it's robust. please see the C & C++ SEH rule stated in patch description section.

Sep 22 2020, 4:20 PM · Restricted Project, Restricted Project

Sep 21 2020

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Thank you for prompt reply again.

Sep 21 2020, 6:11 PM · Restricted Project, Restricted Project

Sep 17 2020

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Hi, John, thank you for reviewing this patch and providing feedback.
regarding your comments:

Sep 17 2020, 4:00 PM · Restricted Project, Restricted Project

Jul 12 2020

tentzen committed rG66f1dcd872db: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally (authored by tentzen).
[Windows SEH] Fix the frame-ptr of a nested-filter within a _finally
Jul 12 2020, 1:40 AM
tentzen closed D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.
Jul 12 2020, 1:40 AM · Restricted Project

Jun 11 2020

tentzen added a comment to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

this patch has lasted for a couple of months. a bug in this area is hard and time-consuming to diagnose.
it's better to get this fix in sooner than later. could someone review and approve it?
thanks,

Jun 11 2020, 4:34 PM · Restricted Project
tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

does it look good to you? or any more comment?
thanks

Jun 11 2020, 4:33 PM · Restricted Project, Restricted Project

Jun 4 2020

tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

update a couple of changes per David Majnemer's suggestion.

Jun 4 2020, 1:02 AM · Restricted Project, Restricted Project

Jun 3 2020

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

thank you David. will update and submit a new patch shortly.

Jun 3 2020, 11:57 PM · Restricted Project, Restricted Project

Jun 2 2020

tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Is there any feedback? thanks,

Jun 2 2020, 3:56 PM · Restricted Project, Restricted Project
tentzen added a comment to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

Hi, does this look good? or is there any other concern?
thanks,

Jun 2 2020, 3:25 PM · Restricted Project

May 26 2020

tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

update LangRef.rst for new intrinsics

May 26 2020, 4:55 PM · Restricted Project, Restricted Project

May 22 2020

tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

fixed tidy warnings

May 22 2020, 5:10 PM · Restricted Project, Restricted Project
tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

It may be helpful (even for the reviewers) to first specify their behavior,
instead of writing that after-the-fact "backwardly" based on the implementation.

May 22 2020, 5:10 PM · Restricted Project, Restricted Project
tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

Per Roman Lebedev's feedback, divide the patch into Clang and LLVM.

May 22 2020, 4:06 PM · Restricted Project, Restricted Project
tentzen added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

This should likely be at least 3 patches: llvm middle-end, llvm codegen, clang.
Langref changes missing for new intrinsics.
Please post all patches with full context (-U99999)

May 22 2020, 3:34 PM · Restricted Project, Restricted Project
tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

avoid lint warnings and lint hang on Windows

May 22 2020, 12:01 AM · Restricted Project, Restricted Project

May 21 2020

tentzen updated the diff for D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

fixed formats

May 21 2020, 1:03 AM · Restricted Project, Restricted Project

May 20 2020

tentzen created D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.
May 20 2020, 5:07 PM · Restricted Project, Restricted Project

May 15 2020

tentzen committed rGe32f8e5d4ae8: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table (authored by tentzen).
[Windows EH] Fix the order of Nested try-catches in $tryMap$ table
May 15 2020, 10:37 PM
tentzen closed D79474: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.
May 15 2020, 10:37 PM · Restricted Project
tentzen added reviewers for D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally: andrew.w.kaylor, pengfei.
May 15 2020, 10:36 PM · Restricted Project

May 13 2020

tentzen added a comment to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

Hi, Is there more concern?
To re-iterate the implementation strategy of this change:

May 13 2020, 11:57 PM · Restricted Project
tentzen added a comment to D79760: [WinEH64] Fix a crush issue when c++ exception nested in a particular form..

right, I noticed that too and looked into it further..

May 13 2020, 11:23 PM · Restricted Project
tentzen updated the diff for D79474: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.

Per discussion in https://reviews.llvm.org/D79760 , x86 test case there failed.

May 13 2020, 11:23 PM · Restricted Project
tentzen added a comment to D79760: [WinEH64] Fix a crush issue when c++ exception nested in a particular form..

I meant did you debug into the crash of your test case in https://bugs.llvm.org/show_bug.cgi?id=42266 ?

May 13 2020, 1:02 AM · Restricted Project

May 12 2020

tentzen added a comment to D79760: [WinEH64] Fix a crush issue when c++ exception nested in a particular form..

Hi, Pengfei,
Did you get to the root of the bug and check the correctness of ctor&dtor ordering on your test cases?
Or you just tried to copy MSVC?

May 12 2020, 11:09 PM · Restricted Project
tentzen added a comment to D79760: [WinEH64] Fix a crush issue when c++ exception nested in a particular form..

Hi, Pengfei,
Is this the same bug as this patch, https://reviews.llvm.org/D79474/new/ , right?

May 12 2020, 7:56 PM · Restricted Project

May 6 2020

tentzen added a comment to D79474: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.

thank you David, good eyes!
Is there any more concern?

May 6 2020, 2:44 PM · Restricted Project
tentzen updated the diff for D79474: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.

fixed a comment.

May 6 2020, 2:44 PM · Restricted Project
tentzen added a comment to D79474: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.

Hi, Reid,
the existing test case is fixed. I think there is no need to add one more unit test for this patch.
thanks,

May 6 2020, 1:34 AM · Restricted Project
tentzen updated the diff for D79474: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.

fix a typo in test case

May 6 2020, 1:34 AM · Restricted Project
tentzen created D79474: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.
May 6 2020, 1:02 AM · Restricted Project

May 3 2020

tentzen committed rG21c1a0c7309a: Test Commit: add two head comments in WinEHPrepare.cpp (authored by tentzen).
Test Commit: add two head comments in WinEHPrepare.cpp
May 3 2020, 1:32 AM

May 1 2020

tentzen added a reviewer for D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally: asl.

Hi, Anton,
I found you are the Code Owner of "Exception handling, Windows CodeGen, ARM EABI".
could you please provide a quick review here? thanks,

May 1 2020, 6:25 PM · Restricted Project

Apr 28 2020

tentzen added a comment to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

any more concern or comment?
thanks,

Apr 28 2020, 4:45 PM · Restricted Project

Apr 16 2020

tentzen added a comment to D77920: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.

right, thank you Reid. I see it now :

Apr 16 2020, 3:37 PM · Restricted Project
tentzen updated the diff for D77936: [Windows SEH] Fix abnormal-exits in _try.

remade a patch after re-sync

Apr 16 2020, 12:47 AM · Restricted Project

Apr 15 2020

tentzen added a comment to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

It can be greater than 2 because this Map includes Decls of User locals from parent.
see CodeGenFunction::EmitCapturedLocals() (the same place of this fix).

..
auto I = ParentCGF.LocalDeclMap.find(VD);
if (I == ParentCGF.LocalDeclMap.end())
  continue;
Apr 15 2020, 11:25 PM · Restricted Project
tentzen added a comment to D77920: [Windows EH] Fix the order of Nested try-catches in $tryMap$ table.
In D77920#1984956, @rnk wrote:

Sounds good, and the fix looks simple, but please add a test.

I'm surprised this change doesn't break an existing test, I'd expect us to have a test for the LSDA of a try-in-catch:

try { }
catch () {
  try { }
  catch () { }
}

As I understand it, the outer TBME must appear before the inner one, and the current behavior puts them in the other way around, innermost first.

Apr 15 2020, 4:34 PM · Restricted Project
tentzen updated the diff for D77936: [Windows SEH] Fix abnormal-exits in _try.

Replace F_HasSehAbnormalExits with F_HasExitSwitch

Apr 15 2020, 2:55 PM · Restricted Project
tentzen added a comment to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

The fix there deals with SEH filter with SEH _finally parent where its prototype is FIXED (2 implicit parameters). It will never change.

Apr 15 2020, 1:46 PM · Restricted Project

Apr 14 2020

tentzen added a comment to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

Searching LocalDeclMap is less problematic, I guess... but still, it should be possible to something more straightforward. Maybe make startOutlinedSEHHelper store the actual ImplicitParamDecl, or something like that.

Apr 14 2020, 4:20 PM · Restricted Project
tentzen added a comment to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

Again, using the name isn't reliable. Among other things, in release builds, IR values don't have names at all.

Apr 14 2020, 1:33 AM · Restricted Project
tentzen updated the diff for D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

Do not use name comparison to locate parent's alloca of frame-pointer-addr.
search parent's LocalDeclMap instead.

Apr 14 2020, 1:33 AM · Restricted Project

Apr 13 2020

tentzen added inline comments to D77936: [Windows SEH] Fix abnormal-exits in _try.
Apr 13 2020, 4:20 PM · Restricted Project
tentzen updated the diff for D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

Remove hard-code name "frame-pointer".
get the name from 2nd Arg of the _finally().

Apr 13 2020, 2:07 AM · Restricted Project
tentzen added inline comments to D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.
Apr 13 2020, 1:35 AM · Restricted Project

Apr 12 2020

tentzen updated the diff for D77936: [Windows SEH] Fix abnormal-exits in _try.

Per Eli's suggestion,
Use icmp (and zext) to check the JumpDest index, instead of directly passing the Index to _finally().

Apr 12 2020, 6:42 PM · Restricted Project
tentzen added a comment to D77936: [Windows SEH] Fix abnormal-exits in _try.

I'm not concerned about the performance implications of whatever approach we take here. In the worst case, an "icmp+zext" corresponds to two extra arithmetic instructions; that's not enough to matter. And I expect usually it'll get optimized away.

I'd prefer to avoid exposing our cleanup numbering to user code, if possible. The Microsoft documentation says it returns 0 or 1.

Apr 12 2020, 6:42 PM · Restricted Project
tentzen updated the diff for D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.

Fixed the format at comment lines

Apr 12 2020, 5:38 PM · Restricted Project
tentzen created D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally.
Apr 12 2020, 4:01 PM · Restricted Project
tentzen added a comment to D77936: [Windows SEH] Fix abnormal-exits in _try.

Instead of asserting there are less than 256 cleanup destinations, can you emit an icmp against zero, or something like that?

Apr 12 2020, 2:25 PM · Restricted Project
tentzen updated the diff for D77936: [Windows SEH] Fix abnormal-exits in _try.

Add option -fms-extensions in test case

Apr 12 2020, 1:02 AM · Restricted Project

Apr 11 2020

tentzen updated the diff for D77936: [Windows SEH] Fix abnormal-exits in _try.

Per Eli's feedbacks:
(1) a test case (windows-seh-abnormal-exit.c) is added under clang\test\CodeGen directory.
(2) an assert is added to catch the extremely rare case that the number of JumpDests in a function is greater than 255. The max. number of JumpDests is 1 (return) + 2*M + N, where M is the number of for/while-loops and N is the number of Goto targets.

Apr 11 2020, 11:57 PM · Restricted Project
tentzen created D77936: [Windows SEH] Fix abnormal-exits in _try.
Apr 11 2020, 1:34 AM · Restricted Project