[PATCH, MIPS] Add support for cins Octeon instruction
Adam Nemet
anemet@caviumnetworks.com
Fri Aug 29 22:21:00 GMT 2008
Richard Sandiford wrote:
> Adam Nemet <anemet@caviumnetworks.com> writes:
>> +/* The canonical form of a mask-low-and-shift-left operation is
>> + (and (ashift X SHIFT) MASK) where MASK has the lower SHIFT number of
>> + bits cleared. Thus we need to shift MASK to the right before
>> + checking if it is a valid mask value. If true return the length of
>> + the mask, otherwise return -1. */
>> +
>> +int
>> +mask_low_and_shift_len (rtx mask, rtx shift)
>> +{
>> + return exact_log2 ((UINTVAL (mask) >> INTVAL (shift)) + 1);
>> +}
>
> Perhaps I'm missing something, but is this really valid for SImode
> when the sum of the mask and shift lengths is 32? Mightn't we get
> something that isn't properly sign-extended?
I thought about this originally, in fact, this is function f in the
second testcase I am adding. My thinking was that assuming the current
implementation of and:SI (generates and/andi) this might not be valid
but then the original expression would not have been valid either (in
the machine instruction sense, not in RTL sense). IOW, if we had:
(and:SI (ashift:SI X 24) (const_int 0xff000000))
and assuming GCC would generate code without removing the redundant
"and" we would generate incorrect code. This is because (and:SI X
0xffffffff) would use the "and" instruction which is 64-bit.
Being forced to think about this once more ;) now I can see that the
logic is flawed and instead and:SI should be fixed as well.
I will change this to not match if the sign bit is effected by the
operation. I can also look at fixing and:SI during stage 3.
> Much more minor, but I've tried to avoid declaration assignments
> in mips.c. (I know this isn't consistent in gcc as a whole.)
> In this case, IN_RANGE_P is specifically designed to evaluate
> the first argument once, so:
>
> return IN_RANGE (mask_low_and_shift_len (mask, shift), 1, maxlen);
>
> avoids the problem while being simpler.
Sure, no problem. Can you provide some insight why decl assignments are
undesired? I consider them misleading within a loop but they seem to
work intuitively outside of loops.
> I'm worried that:
>
>> + return exact_log2 ((UINTVAL (mask) >> INTVAL (shift)) + 1);
>
> is undefined for "out-of-range" shift values. There's no requirement
> for rtl code to canonicalise the shift amount; see the any_shift
> insns for what we have to do there.
Sure.
I will resubmitted once we agree on the SI question.
Adam
More information about the Gcc-patches
mailing list