On 08/13/2012 10:07 AM, Ulrich Weigand wrote: >> +/* Check whether a rotate of ROTL followed by an AND of CONTIG is equivalent >> + to a shift followed by the AND. In particular, CONTIG should not overlap >> + the (rotated) bit 0/bit 63 gap. */ >> + >> +bool >> +s390_extzv_shift_ok (int bitsize, int rotl, unsigned HOST_WIDE_INT contig) >> +{ >> + int pos, len; >> + bool ok; >> + >> + ok = s390_contiguous_bitmask_p (contig, bitsize, &pos, &len); >> + gcc_assert (ok); >> + >> + return (rotl <= pos || rotl >= pos + len + (64 - bitsize)); >> +} > > I don't quite see how this can work correctly for both left and right shifts. > E.g. for bitsize == 64, rotl == 1, contig == 1, a left shift by 1 followed > by the AND is always zero, and certainly not equal to the rotate. But the > routine would return true. (Note that the same routine would be called for > a *right* shift by 63, in which case the "true" result is actually correct.)
Absolutely correct; I hadn't considered that. >> +(define_insn "extzv" >> + [(set (match_operand:DI 0 "register_operand" "=d") >> + (zero_extract:DI >> + (match_operand:DI 1 "register_operand" "d") >> + (match_operand 2 "const_int_operand" "") >> + (match_operand 3 "const_int_operand" ""))) >> + (clobber (reg:CC CC_REGNUM))] >> + "TARGET_Z10" >> + "risbg\t%0,%1,63-%3-%2,128+63,63-%3-%2" > > This doesn't look right for bits-big-endian order. Shouldn't we have a > rotate count of %3+%2, and a start bit position of 64-%2 ? Yes. >> -(define_insn_and_split "*extv<mode>" >> +(define_insn_and_split "*pre_z10_extv<mode>" >> [(set (match_operand:GPR 0 "register_operand" "=d") >> (sign_extract:GPR (match_operand:QI 1 "s_operand" "QS") >> - (match_operand 2 "const_int_operand" "n") >> + (match_operand 2 "nonzero_shift_count_operand" "") >> (const_int 0))) >> (clobber (reg:CC CC_REGNUM))] >> - "INTVAL (operands[2]) > 0 >> - && INTVAL (operands[2]) <= GET_MODE_BITSIZE (SImode)" >> + "!TARGET_Z10" >> "#" >> "&& reload_completed" > > Why disable this for pre-z10? I blatantly assumed that L+RISBGZ was better than ICM+SRL. I probably shouldn't have also disabled the sign_extract version as well. > While those are equivalent (given that TARGET_Z10 implies TARGET_ZARCH), > it would seem more in line with many other patterns to use GPR instead of DSI. Ok. >> + && INTVAL (operands[3]) == 64 - INTVAL (operands[1]) - INTVAL >> (operands[2])" >> + "rnsbg\t%0,%4,%2,%2+%1-1,64-%2,%1" > > I guess the last "," is supposed to be a "-". (Then we might > as well use %3 instead of 64-%2-%1.) Good catch. r~