This is an archive of the discontinued LLVM Phabricator instance.

[IROutliner] Deduplicating functions that only require inputs.
ClosedPublic

Authored by AndrewLitteken on Sep 1 2020, 1:31 PM.

Details

Summary

Extracted regions can have both inputs and outputs. In addition, the CodeExtractor removes inputs that are only used in llvm.assumes, and sunken allocas (values are used entirely in the extracted region as denoted by lifetime intrinsics). We also cannot combine sections that have different constants in the same structural location, and these constants will have to elevated to argument. This patch deduplicates extracted functions that only have inputs and non of the special cases.

Diff Detail

Event Timeline

AndrewLitteken created this revision.Sep 1 2020, 1:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 1 2020, 1:32 PM
AndrewLitteken added a reviewer: paquette.

Updating for clang-format.

Updating diff to reflect changes in other patches.

There's quite a few dead variables added in this patch. Do they belong in later patches in the series?

llvm/lib/Transforms/IPO/IROutliner.cpp
267

non-rhetorical question: should it be marked noduplicate too?

280

dead variable

383

ArrayRef

386

elsewhere you have usually called them IRSC

398–400
408

I think this needs to be folded into the assertion to avoid -Wundef in Release builds.

412

emplace

447

both of these variables are dead

513–515

Unless you need to filter them for some reason?

532

variable is dead

534

I'd sink this to where it's used.

539

Why is the first one skipped, and can this be a range-for?

A lot of the dead variables belong to future patches, I'll make sure to weed them out in the next diff.

llvm/lib/Transforms/IPO/IROutliner.cpp
267

I'm not entirely sure, it seems like it might be a good idea for safety.

539

The first extracted function is used to construct the overall outlined function, so it is handled slightly differently.

This revision is now accepted and ready to land.Sep 18 2020, 10:44 PM

Updating to reflect other patches.

The URL was not included in commit message for 5cdc4f57e50bbe0d211c109517c17defe78e0b73. Closing manually.