[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

[Chris Lattner]

Apologies for the last email being rambling, and in advance for this one also being rambling. :)

My basic perspective is that I want to sink as much logic into the target-independent code as possible and have the fewest number of target hooks reasonable.  This is for three reasons:

1. Porting to a new target is easier with fewer target hooks, and they get better codegen 'out of the box' by specifying primitives instead of having to tweak tons of knobs.

2. A single and well tuned implementation of something in target independent code is better than having a few well tuned impls in some targets and missing or just wrong impls in others.  For example, this xform is definitely profitable on X86 chips as well, probably many others.

3. Less target-specific code to maintain and push around as the compiler moves forward.

anyway, these are just general principles why I don't like new target hooks when they can be avoided.  More details:

> 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.

Right.  However, the implementation for ARM doesn't consider this at all.  Also, this cost is not something that should need target-specific information to quantify.

>>  (And, at least for duplicating
>> indirect branches on ARM Cortex processors, we don't want to limit
>> that.)

Why not??

>>  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.

They are fairly common in switch statements that codegen to jump tables, but not anyway near as common as normal branches.

>> Most modern high-performance processors will benefit from tail
>> duplicating indirect branches, so that would be another reason to
>> avoid the target hook.

Right.  I would also like the target hook much better if it was of the form "DoesBranchPredictionOfIndirectCallsBasedOnPC" or something like that.  This predicate makes it *really really* obvious what it doing.  By returning a 'cost' with no real units, the target author has no "truth" they just fiddle around until the 'right thing happens'.  Given the documentation, it's very unclear (as a target author) why you'd want to specify the hook you added.

I much prefer very simple hooks that return an _aspect of the micro architecture_ rather than having hooks that _implement pieces of codegen passes_.  The later is the approach that GCC took and is one of the major reasons it is difficult to change the algorithms in the GCC backend.

>> 
>> 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.

Using the extant isIndirectBranch flag would be best, but even adding this sort of target hook would be somewhat ok.  At least this would be a property of the architecture.  If we can avoid it, I'd definitely prefer to of course.

Dan wrote:
>  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.

What is the units that it returns?  As a target author, how do I know to return +2 or +4 or +1234?

Thanks for working on this Bob!

-Chris