[all-commits] [llvm/llvm-project] f6795e: [CodeExtractor] Refactor extractCodeRegion, fix al...

[Michael Kruse via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: f6795e6b4f619cbecc59a92f7e5fad7ca90ece54
      https://github.com/llvm/llvm-project/commit/f6795e6b4f619cbecc59a92f7e5fad7ca90ece54
  Author: Michael Kruse <llvm-project at meinersbur.de>
  Date:   2024-11-12 (Tue, 12 Nov 2024)

  Changed paths:
    M llvm/include/llvm/Transforms/Utils/CodeExtractor.h
    M llvm/lib/Transforms/Utils/CodeExtractor.cpp
    M llvm/unittests/Transforms/Utils/CodeExtractorTest.cpp

  Log Message:
  -----------
  [CodeExtractor] Refactor extractCodeRegion, fix alloca emission. (#114419)

Reorganize the code into phases:

 * Analyze/normalize
 * Create extracted function prototype
 * Generate the new function's implementation
 * Generate call to new function
 * Connect call to original function's CFG

The motivation is #114669 to optionally clone the selected code region
into the new function instead of moving it. The current structure made
it difficult to add such functionality since there was no obvious place
to do so, not made easier by some functions doing more than their name
suggests. For instance, constructFunction modifies code outside the
constructed function, but also function properties such as
setPersonalityFn are derived somewhere else. Another example is
emitCallAndSwitchStatement, which despite its name also inserts stores
for output parameters.

Many operations also implicitly depend on the order they are applied
which this patch tries to reduce. For instance, ExtractedFuncRetVals
becomes the list exit blocks which also defines the return value when
leaving via that block. It is computed early such that the new
function's return instructions and the switch can be generated
independently. Also, ExtractedFuncRetVals is combining the lists
ExitBlocks and OldTargets which were not always kept consistent with
each other or NumExitBlocks. The method recomputeExitBlocks() will
update it when necessary.

The coding style partially contradict the current coding standard. For
instance some local variable start with lower case letters. I updated
some, but not all occurrences to make the diff match at least some lines
as unchanged.

The patch [D96854](https://reviews.llvm.org/D96854) introduced some
confusion of function argument indexes this is fixed here as well, hence
the patch is not NFC anymore. Tested in modified CodeExtractorTest.cpp.
Patch [D121061](https://reviews.llvm.org/D121061) introduced
AllocationBlock, but not all allocas were inserted there.

Efectively includes the following fixes:
1. https://github.com/llvm/llvm-project/commit/ce73b1672a6053d5974dc2342881aac02efe2dbb
2. https://github.com/llvm/llvm-project/commit/4aaa92578686176243a294eeb2ca5697a99edcaa
3. Missing allocas, still unfixed

Originally submitted as https://reviews.llvm.org/D115218



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications