[PATCH, MIPS] Add bbit* Octeon instructions

[Adam Nemet]

Richard Sandiford wrote:
> Adam Nemet <anemet@caviumnetworks.com> writes:
>> If that's acceptable I'd like to sort out whether we need the SI-mode
>> extraction while fixing this miscompilation.  And then depending on the
>> outcome also remove SI mode from the bbit pattern.
> 
> Won't you still need SImode for 32-bit targets?  (I realise you're
> planning on removing the o32 multilib, but still... it seems odd
> remove SImode stuff from 64-bit targets.)

What I meant was to remove the SI pattern for TARGET_64BIT not for
!TARGET_64BIT (o32).  Basically writing these patterns with a new mode
iterator:

  (define_mode_iterator WORD [(SI "!TARGET_64BIT") (DI "TARGET_64BIT)])

(In fact I had a parallel version of this patch written and tested with
WORD and the bbit tests were passing with o32 and I had the same number
of bbit instructions in libgcj.so.)

>> @@ -852,12 +866,21 @@
>>  ;; .........................
>>  
>>  (define_delay (and (eq_attr "type" "branch")
>> -		   (eq (symbol_ref "TARGET_MIPS16") (const_int 0)))
>> +		   (and (eq (symbol_ref "TARGET_MIPS16") (const_int 0))
>> +			(eq_attr "has_likely" "yes")))
>>    [(eq_attr "can_delay" "yes")
>>     (nil)
>>     (and (eq_attr "branch_likely" "yes")
>>  	(eq_attr "can_delay" "yes"))])
>>  
>> +;; Branches that don't have likely variants do not annul on false.
>> +(define_delay (and (eq_attr "type" "branch")
>> +		   (and (eq (symbol_ref "TARGET_MIPS16") (const_int 0))
>> +			(eq_attr "has_likely" "no")))
>> +  [(eq_attr "can_delay" "yes")
>> +   (nil)
>> +   (nil)])
>> +
> 
> Can't you simply use:
> 
>   (set_attr "branch_likely" "no")
> 
> avoiding the extra define_delay and "has_likely" attribute?

No and I should have really explained that in the email because this had
been my first reaction as well.  The annul predicates test the
*candidate instruction* for the delay slot and *not* the branch
instruction.  Only the actual define_delay predicate is a test on the
branch instruction so the check needs to go there.

> I'd prefer "const_int_operand" "" over "immediate_operand" "I".
> (The second version probably still appears in mips.md, but it's
> technically bad to have predicates that only match constants,
> and whose constraints disallow things that the predicates allow.)

I see.  Thanks for the explanation.  I made the change.

> I don't know whether a range check is needed here or not, but since
> this area seems a little under-specified, it might be safer to add one:
> 
>   "ISA_HAS_BBIT && UINTVAL (operands[2]) < GET_MODE_BITSIZE (<GPR>mode)"

I added that too, can't hurt.

> OK with those changes, thanks.

Let me know if you agree with the has_likely explanation.  Attached is
what I have successfully rebootstrapped and retested.

Adam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bbit-2.patch
Type: text/x-patch
Size: 7300 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20080828/4921b0b0/attachment.bin>