[PATCH] D141916: WIP: Unwindabort: add support for IR transforms and analysis.

[James Y Knight via Phabricator via llvm-commits]

jyknight marked 8 inline comments as done.
jyknight added a comment.
Herald added a subscriber: hoy.

In D141916#4059553 <https://reviews.llvm.org/D141916#4059553>, @lebedev.ri wrote:

> This is missing a very important correctness change to `haveSameSpecialState()`/`FunctionComparator::cmpOperations()`

Yikes -- thanks for pointing that out. It's quite unfortunate that we hide equality and comparisons for Instruction types in 2 other non-obvious places. It'd be great to move all of that into methods _on_ the type, so it's more obvious that it needs to be changed when adding data to an instruction. It looks like I don't need to change FunctionComparator, since it just generically checks getRawSubclassData() for equality. But, I do need to modify hasSameSpecialState, since it does not do so (yikes^2 that these functions differ in that way!).

I'll add those changes to  into the previous 3 patches in this series.



================
Comment at: llvm/include/llvm/Transforms/Utils/Local.h:357
 
+enum class RemoveUnwindEdgeMode {
+  Normal,
----------------
lebedev.ri wrote:
> ?
I'll keep the current name, in order to make clear this enum is intended to be specific for this function's argument, rather than general-purpose.


================
Comment at: llvm/test/Transforms/FunctionAttrs/nounwind.ll:85
+; "catch_thing", because the call to @__cxa_end_catch is not marked
+; nounwind. TODO: _could_ it be marked nounwind?
+
----------------
barannikov88 wrote:
> > _could_ it be marked nounwind?
> 
> In general -- no. One has to prove that the destructor of the caught exception does not throw.
> 
Ah, right. TODO removed, explanation added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141916