[llvm] r310894 - [MachineOutliner] Only outline candidates of length >= 2

[Davide Italiano via llvm-commits]

On Mon, Aug 14, 2017 at 3:57 PM, Jessica Paquette via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: paquette
> Date: Mon Aug 14 15:57:41 2017
> New Revision: 310894
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310894&view=rev
> Log:
> [MachineOutliner] Only outline candidates of length >= 2
>
> Since we don't factor in instruction lengths into outlining calculations
> right now, it's never the case that a candidate could have length < 2.
>
> Thus, we should quit early when we see such candidates.
>
>
> Modified:
>     llvm/trunk/lib/CodeGen/MachineOutliner.cpp
>
> Modified: llvm/trunk/lib/CodeGen/MachineOutliner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineOutliner.cpp?rev=310894&r1=310893&r2=310894&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineOutliner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineOutliner.cpp Mon Aug 14 15:57:41 2017
> @@ -844,6 +844,13 @@ MachineOutliner::findCandidates(SuffixTr
>
>      // Figure out if this candidate is beneficial.
>      size_t StringLen = Leaf->ConcatLen - Leaf->size();
> +
> +    // Too short to be beneficial; skip it.
> +    // FIXME: This isn't necessarily true for, say, X86. If we factor in
> +    // instruction lengths we need more information than this.
> +    if (StringLen < 2)
> +      continue;
> +

As your comment hints (and as I expect) this is very tricky to get
right without some additional per-target information. Rather than
having this check living in the generic code, have you considered
adding a per-target callback? (I'm not a huge fan of those), but it
seems the best option until we have a better heuristic.
Also, nit, can you have StringLen == 0 ? If not, you can just
assert(StringLen > 0) somewhere and here bail out if StringLen == 1.

Thanks,

--
Davide