[PATCH] D115216: [CodeExtractor] Optionally keep code in original function.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In D115216#3281303 <https://reviews.llvm.org/D115216#3281303>, @jdoerfert wrote:

> In my mind that use case looked something like:
> When createParallel starts we push new special blocks (EHResumeBlock, ...) onto the stack. 
> All cleanup uses the associated special blocks while body generation happens. 
> Once the region is done we revert back to the special blocks that were used before.
>
> Why doesn't this work? I thought it already did.

Unfortunately is it a lot more complicated. There is no limited list of blocks that might be shared between outlined and outer function, the special blocks in CodeGenFunction (EHResumeBlock, TerminateLandingPad, TerminateHandler, UnreachableBlock) are just the most obvious ones. For instance, cleanup code is emitted only once and looks like this:

  bb1:
    store i32 1, %c
    br label %common_cleanup
  
  bb2:
    store i32 2, %c
    br label %common_cleanup
  
  common_cleanup:
    [cleanup code]
    switch i32 %c, [
      [i32 1, label %bb1_continue],
      [i32 2, label %bb2_continue]
    }
  
  bb1_continue:
    [...]
  
  bb2_continue:
    [...]

And for every destruction a new switch case is added. Similar things happen with exceptions e.g. landing pads. What's worst, I have no exhaustive list of circumstances when sharing of code may happen and there is also the risk that the switched-out special block are stored in some other data structure and re-used after having left the region. I am not saying these cannot be fixed by switching-out some fields during the outlined region (there's also TerminateFunclets, ExceptionSlot, EHSelectorSlot, SEHCodeSlotStack, SEHInfo, ObjCEHValueStack, CurFuncIsThunk, ...), up to the entire CodeGenFunction but I vastly prefer a straightforward solution over making Clang's internals even more complicated. It's also one less concern for potential other front-ends than Clang/Flang.

That being said, a clean exception handling would indeed create a new handling context; currently exception handling inside the outlined region will also branch to the cleanup code in the outer function using the dtor pattern above. Actually triggering the exception at runtime is not well-formed in OpenMP, but the compiler should not crash crash because the cleanup code has been moved away. That is, copying the code instead of moving it will already allow us to compile C++ code without `-fno-exceptions`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115216