[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D141918#4577930 , @jyknight wrote:

> In D141918#4566838 , @smeenai wrote:
>
>> $ clang -std=c++20 -O2 -c crash.cpp
>> cannot use musttail with unwindabort
>
> Thanks for the report. We were missing a check of `CI.isUnwindAbort()` in 
> CoroSplit.cpp's `shouldBeMustTail()` function -- fixed for next version of 
> the series. (It's not a regression in tail-callability for coroutines, since 
> that `call unwindabort` would've previously been an `invoke`, which also 
> cannot be musttail'd.)

Just to confirm, this hasn't been uploaded yet, right? I wanted to try it out 
once it is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141918/new/

https://reviews.llvm.org/D141918

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D141918#4566838 , @smeenai wrote:

> $ clang -std=c++20 -O2 -c crash.cpp
> cannot use musttail with unwindabort

Thanks for the report. We were missing a check of `CI.isUnwindAbort()` in 
CoroSplit.cpp's `shouldBeMustTail()` function -- fixed for next version of the 
series. (It's not a regression in tail-callability for coroutines, since that 
`call unwindabort` would've previously been an `invoke`, which also cannot be 
musttail'd.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141918/new/

https://reviews.llvm.org/D141918

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Thanks for the rebase!

We're eager to try this for Meta's Android apps, where size is a priority and 
better `noexcept` codegen would be really helpful. I ran into a crash with 
coroutines though, which blocks me from fully evaluating the size difference 
made by these changes. The reduced test case is in P8313 
, and you can repro the crash by running:

  $ clang -std=c++20 -O2 -c crash.cpp
  cannot use musttail with unwindabort
musttail call unwindabort fastcc void %10(ptr %call57)
  in function _Z5startv.resume
  fatal error: error in backend: Broken function found, compilation aborted!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141918/new/

https://reviews.llvm.org/D141918

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 546613.
jyknight edited the summary of this revision.
jyknight added a comment.

Rebase patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141918/new/

https://reviews.llvm.org/D141918

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGCleanup.h
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/EHScopeStack.h
  clang/test/CXX/except/except.spec/p9-dynamic.cpp
  clang/test/CXX/except/except.spec/p9-noexcept.cpp
  clang/test/CodeGenCXX/arm-generated-fn-attr.cpp
  clang/test/CodeGenCXX/block-byref-cxx-objc.cpp
  clang/test/CodeGenCXX/clang-call-terminate.uwtable.cpp
  clang/test/CodeGenCXX/cxx0x-delegating-ctors.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp
  clang/test/CodeGenCXX/debug-info-line.cpp
  clang/test/CodeGenCXX/destructors.cpp
  clang/test/CodeGenCXX/dllimport-runtime-fns.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/lambda-expressions.cpp
  clang/test/CodeGenCXX/noexcept.cpp
  clang/test/CodeGenCXX/nrvo.cpp
  clang/test/CodeGenCXX/partial-destruction.cpp
  clang/test/CodeGenCXX/pod-member-memcpys.cpp
  clang/test/CodeGenCXX/pr58798.cpp
  clang/test/CodeGenCXX/rtti-qualfn.cpp
  clang/test/CodeGenCXX/runtime-dllstorage.cpp
  clang/test/CodeGenObjCXX/arc-blocks.mm
  clang/test/CodeGenObjCXX/literals.mm
  clang/test/OpenMP/atomic_codegen.cpp
  clang/test/OpenMP/critical_codegen.cpp
  clang/test/OpenMP/critical_codegen_attr.cpp
  clang/test/OpenMP/declare_reduction_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/for_codegen.cpp
  clang/test/OpenMP/for_simd_codegen.cpp
  clang/test/OpenMP/masked_codegen.cpp
  clang/test/OpenMP/master_codegen.cpp
  clang/test/OpenMP/ordered_doacross_codegen.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_for_codegen.cpp
  clang/test/OpenMP/parallel_for_simd_codegen.cpp
  clang/test/OpenMP/parallel_master_codegen.cpp
  clang/test/OpenMP/parallel_sections_codegen.cpp
  clang/test/OpenMP/sections_codegen.cpp
  clang/test/OpenMP/simd_codegen.cpp
  clang/test/OpenMP/single_codegen.cpp
  clang/test/OpenMP/taskgroup_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:5391
   if (FPtr->hasFnAttribute(llvm::Attribute::NoUnwind))
 CannotThrow = true;
+

Is there coverage for a nounwind callee + an unwindabort call?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141918/new/

https://reviews.llvm.org/D141918

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.
Herald added a subscriber: jplehr.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:2021
+  /* Indicates whether we should generate calls using unwindabort, instead of a
+   * terminate landingpad, for the current EH Scope. */
+  bool shouldUseUnwindAbort() const;

Other method docs in the file use `///`-style doxygen comments.



Comment at: clang/test/CodeGenCXX/exceptions.cpp:81
 
-  //   rdar://11904428
-  //   Terminate landing pads should call __cxa_begin_catch first.

FWI(was)W, this radar points at:  https://bugs.llvm.org/show_bug.cgi?id=11893


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141918/new/

https://reviews.llvm.org/D141918

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

I couldn't find an example of creating "resume unwindabort" in the patch 
series. Is it yet to be done?
Could you show a source code example where such an instruction should be 
generated? (By the frontend or an IR pass.) I couldn't get it from the RFC.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141918/new/

https://reviews.llvm.org/D141918

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits