[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz added a comment.

Hi @theraven thanks for reviewing!



================
Comment at: llvm/lib/IR/IntrinsicInst.cpp:61
+  case Intrinsic::objc_sync_enter:
+  case Intrinsic::objc_sync_exit:
+    return true;
----------------
theraven wrote:
> I'm curious why memcpy and friends don't need to be on this list.  Do they get expanded at a different point?  Or is it that the front end inserts a memcpy call and the optimiser that turns them into intrinsics and back preserves operand bundles?  It would be a good idea to have a comment explaining why these specific ones are on the list and not other libc (or math library) functions that may appear in cleanup blocks.  From the code in `InlineFunction.cpp`, it looks as if this is a problem only for intrinsics that may throw, which excludes most of the C standard ones?
The name `mayLowerToFunctionCall` was proposed in a precursor to this review: https://reviews.llvm.org/D127962#inline-1227485. I agree that the term might appear overly general at first. I still followed the advice because:
(1) This controls assignment and propagation of operand bundles, which is a very general concept.
(2) From the perspective of IR transformations, only a few intrinsics actually lower to function calls (the majority remains intrinsics).
(3) The approach might be applied to other intrinsics, e.g. statepoints

However, this is not obvious and because other readers might have the same question, I extended my comment in the header like this:
```
Check if the intrinsic might lower into a regular function call in the course of IR transformations
```

Do you think that makes it more clear?

**For additional context**
Right now the list only contains ObjC ARC intrinsics. These are subject to `PreISelIntrinsicLowering` which means they are lowered to regular function calls before we run exception handling passes:
https://github.com/llvm/llvm-project/blob/c73adbad6a9964e0700865b7c786cc6885898c68/llvm/lib/CodeGen/TargetPassConfig.cpp#L1114-L1118

WinEHPrepare is an exception handling pass and it accepts call instructions in EH funclets only if they: 
* target nounwind intrinsics or inline assembly
* have a matching funclet bundle operand

It declares other calls as "implausible instructions" and marks them unreachable, which caused the truncation bug that this patch aims to fix.

All ObjC ARC intrinsics have the `nounwind` attribute. My understanding is that the only user-code that might run in turn of an invocation can be destructors -- and they don't throw per definition.

The reason libc functions like memcpy are not in this list is that they are lowered at a later point in time. In the context of WinEHPrepare (and opt in general) they are always intrinsics. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128190