[CodeExtractor] Optionally keep code in original function. #114669
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-keep-blocks. Make the interface easier and --bb-keep-functions alone can result in invalid IR.
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
…/meinersbur/irbuilder-extract
…einersbur/irbuilder-extract-refactor
…inersbur/irbuilder-extract-refactor
…nersbur/irbuilder-extract
…inersbur/irbuilder-extract
…inersbur/irbuilder-extract
Meinersbur
added a commit
that referenced
this pull request
Nov 12, 2024
…4419) 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. ce73b16 2. 4aaa925 3. Missing allocas, still unfixed Originally submitted as https://reviews.llvm.org/D115218
Base automatically changed from
users/meinersbur/irbuilder-extract-refactor
to
main
November 12, 2024 19:12
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When extracting a region into a new function, optionally allow cloning basic blocks and instructions into the extracted function instead of moving them. The keeps the original code in the original function such that they can still be referenced -- and branched to -- in the original function.
The motivation is the use of CodeExtractor in the OpenMPIRBuilder. The implementation of createParallel first emits the parallel region into the lexical function, then uses CodeExtractor to outline that region into a new function. The problem here is that Clang's code generator will reference some basic blocks for code inside as well as outside the region. This includes some special purpose block (EHResumeBlock, TerminateLandingPad, TerminateHandler, UnreachableBlock, ...) and cleanup/dtor code that is re-used from multiple scopes (see test case extract-block-cleanup.ll). Moving these blocks into a different function will result in malformed IR. The KeepOldBlocks option will instead clone the outlined code into a new function keeping the auxiliary code intact, relying on later DCE to remove code that indeed has become unreachable. Additionally, this code could also be uses as a fallback when threading/offloading is disabled via environment option.
Use of KeepOldBlocks by OpenMPIRBuilder is not part of this patch. For testing, we extend llvm-extract allowing the use of this option and thus making it more powerful.
PR Stack:
Originally submitted as https://reviews.llvm.org/D115216