- User Since
- Apr 10 2020, 4:29 PM (41 w, 2 d)
Dec 17 2020
Hi, Is there any more concerns, feedbacks or suggestions?
Nov 24 2020
Nov 19 2020
Per Joseph's feedback, further clarify the semantic of llvm.seh.try.begin and llvm.seh.try.end.
Fixed the clang fault when built without '-feh-asynch' option.
EHStack's CGF field must be initialized even without -feh-asynch.
Nov 18 2020
per review feedback from John McCall and others, this update includes:
Sep 28 2020
Summarize our discussion above, I will follow up with:
Sep 24 2020
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 23 2020
OK, good to know this. thank you!
Other than blocking some opts, does this returns_twice attribute have some other implications?
Sep 22 2020
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 21 2020
Thank you for prompt reply again.
Sep 17 2020
Hi, John, thank you for reviewing this patch and providing feedback.
regarding your comments:
Jul 12 2020
Jun 11 2020
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?
does it look good to you? or any more comment?
Jun 4 2020
update a couple of changes per David Majnemer's suggestion.
Jun 3 2020
thank you David. will update and submit a new patch shortly.
Jun 2 2020
Is there any feedback? thanks,
Hi, does this look good? or is there any other concern?
May 26 2020
update LangRef.rst for new intrinsics
May 22 2020
fixed tidy warnings
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.
Per Roman Lebedev's feedback, divide the patch into Clang and LLVM.
avoid lint warnings and lint hang on Windows
May 21 2020
May 20 2020
May 15 2020
May 13 2020
Hi, Is there more concern?
To re-iterate the implementation strategy of this change:
right, I noticed that too and looked into it further..
Per discussion in https://reviews.llvm.org/D79760 , x86 test case there failed.
I meant did you debug into the crash of your test case in https://bugs.llvm.org/show_bug.cgi?id=42266 ?
May 12 2020
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?
Is this the same bug as this patch, https://reviews.llvm.org/D79474/new/ , right?
May 6 2020
thank you David, good eyes!
Is there any more concern?
fixed a comment.
the existing test case is fixed. I think there is no need to add one more unit test for this patch.
fix a typo in test case
May 3 2020
May 1 2020
I found you are the Code Owner of "Exception handling, Windows CodeGen, ARM EABI".
could you please provide a quick review here? thanks,
Apr 28 2020
any more concern or comment?
Apr 16 2020
right, thank you Reid. I see it now :
remade a patch after re-sync
Apr 15 2020
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;
Replace F_HasSehAbnormalExits with F_HasExitSwitch
The fix there deals with SEH filter with SEH _finally parent where its prototype is FIXED (2 implicit parameters). It will never change.
Apr 14 2020
Do not use name comparison to locate parent's alloca of frame-pointer-addr.
search parent's LocalDeclMap instead.
Apr 13 2020
Remove hard-code name "frame-pointer".
get the name from 2nd Arg of the _finally().
Apr 12 2020
Per Eli's suggestion,
Use icmp (and zext) to check the JumpDest index, instead of directly passing the Index to _finally().
Fixed the format at comment lines
Add option -fms-extensions in test case
Apr 11 2020
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.