[llvm-commits] [llvm] r89187 - in /llvm/trunk: include/llvm/Target/TargetInstrInfo.h lib/CodeGen/BranchFolding.cpp lib/Target/ARM/ARMBaseInstrInfo.cpp lib/Target/ARM/ARMBaseInstrInfo.h lib/Target/ARM/ARMSubtarget.cpp lib/Target/ARM/ARMSubtarget.h

[Dan Gohman]

On Nov 18, 2009, at 9:29 AM, Bob Wilson wrote:

> The only disadvantage of what you suggest is code size.  There is
> currently no limit on the number of predecessors where a block may be
> duplicated, so the effect of duplicating a small block can be
> magnified in the overall code size.  (And, at least for duplicating
> indirect branches on ARM Cortex processors, we don't want to limit
> that.)  On a small low-power device without sophisticated branch
> prediction, where code size typically matters more than usual, we'll
> be doing the wrong thing.  But, indirect branches are not very common,
> so it probably doesn't really matter so much.
>
> Most modern high-performance processors will benefit from tail
> duplicating indirect branches, so that would be another reason to
> avoid the target hook.
>
> Unless someone else has another idea, I'll get rid of the tail
> duplication target hook.  As you mention, we'll need a way to identify
> indirect branches.  I'd prefer to add a new IsIndirectBranch target
> hook.  This goes against your desire to avoid new target hooks, but
> it's nice and simple.  Alternatively, we could add another bool
> reference argument to AnalyzeBranch.  When AnalyzeBranch returns true,
> which currently means that it cannot understand the branch, it would
> set the new argument to indicate whether it is an indirect branch.
> That doesn't seem to fit very well with the AnalyzeBranch interface,
> which is already pretty complicated, so I don't like that so much.

TargetInstrDesc has an isIndirectBranch flag, so you could use
that if you want. However I don't know what the big problem is
with the target hook; it's neither arbitrary nor unitless. Perhaps
what he really meant is that the present use for the hook is too
obscure to justify having the clutter of the hook in TargetInstrInfo,
but I'm just guessing here.

Dan

>
> On Nov 18, 2009, at 8:39 AM, Chris Lattner wrote:
>
>> I understand that this is important for performance, but why isn't
>> this a win (or at least not a loss) for all targets?  Would it be
>> reasonable to just increase the threshold (in the taildup code
>> itself) for unconditional branches to blocks that end with an
>> indirect jump regardless of CPU and whether the jump came from a
>> switch or indirect goto?
>>
>> I guess what I'm getting at is that I'd prefer to not add yet-
>> another target hook (particularly one where targets return an
>> arbitrary and unitless value), and just make analyze branch smarter
>> if needbe.  If the goal is to taildup indirect branches, it would be
>> better to look for that target independent structure rather than
>> foist the issue onto target authors.
>>
>> -Chris