[llvm-commits] CVS: llvm/lib/Target/ARM/ARMInstrInfo.cpp ARMInstrInfo.h ARMTargetMachine.cpp ARMTargetMachine.h

[Chris Lattner]

On May 16, 2007, at 1:02 AM, Evan Cheng wrote:
>> I don't think there is any reason for this to be virtual and target-
>> specific.  In particular, can you please model B/tB the same way PPC
>> handles branches (where an uncond branch is just a conditional branch
>> where the condition is set to 'always')?  That way, you don't need a
>> special case, isPredicatable doesn't need to be virtual, and each
>> target doesn't need to implement PredicateInstruction.
>
> Hrm? PPC has two separate opcodes for conditional and unconditional
> branches.
>
> Are you talking about not having separate opcodes representing
> conditional and unconditional branches? I suppose that's ok except
> for some additional complexity added to AnalyzeBranch (and the fact
> it's harder to read the debug output). I'd prefer to keep the two
> separate (but ok with giving ARM::B a ''always' predicate).

You're right.  We don't want to merge conditional and unconditional  
branches, at least on thumb.  On thumb, conditional and unconditional  
branches have a different size for their displacement field, and a  
different encoding.  Modeling them as the same instruction would be  
wrong.

> But even then I am not sure if that eliminate the need to have a
> virtual function isPredicatable. Conditional branches probably would
> require special handling. Also, while it isn't possible now, it's
> conceivable some instruction predicate field may not be 'always'
> coming into the if-conversion pass. In that case, it would require a
> target hook to examine the field anyway.

I realized this morning what I really meant :).  I think that  
isPredicable should be a non-virtual function that checks TID- 
 >Flags&M_PREDICATED (Actually, it might make sense to introduce a  
new M_PREDICABLE flag).  Further, the default implementation of  
TII::PredicateInstruction should work for all standard instructions,  
but should remain virtual.

A target like arm that has a few special cases (e.g. tB), should mark  
tB explicitly as M_PREDICABLE, and specialize PredicateInstruction.   
The ARM version of PredicateInstruction would handle tB itself (e.g.  
switching it to tBcc), or call the default impl for other cases.

Does that sound reasonable?  That keeps the TII::isPredicable()  
predicate (haha) very fast, yet allows full generality for targets to  
do funny things.

-Chris