Andreas Krebbel wrote: > On 11/30/2015 04:11 PM, Dominik Vogt wrote: > > The attached patch fixes some warnings generated by the setmem... > > patterns in s390.md during build and add test cases for the > > patterns. The patch is to be added on to p of the movstr patch: > > https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03485.html > > > > The test cases validate that the patterns are actually used, but > > at the moment the setmem_long_and pattern is never actually used > > and thus the test case would fail. So I've split the patch in two > > (both attached to this message) to activate this part of the test > > once we've fixed that. > > > > The patch has passed the SPEC2006 testsuite without any measurable > > changes in performance. > > Shouldn't we instead describe the whole setmem operation as unspec including > the other operands as > well? The semantics of the introduced UNSPEC_P_TO_BLK operation is not clear > to me. It suggests to > be some kind of "cast" which it isn't. In fact it is not able to do its job > without the length which > is specified as use outside the unspec.
Well, I guess I suggested to Dominik to leave the basic [parallel (set (dst:BLK) (src:BLK)) (use (length)] structure in place; my understanding is that the middle-end recognizes this as a block move. As "source" in this case we'd use a BLKmode operand that consist iof the same byte replicated a number of times. If we were to use just a single UNSPEC, how would we indicate to the middle-end that a block of memory is modified, without using too coarse- grained clobbers? However, I agree that UNSPEC_P_TO_BLK really should also get the length as input, to make it have precisely defined semantics. Also, I'd rather use a more descriptive name, like UNSPEC_REPLICATE_BYTE or the like. What would you think about something like the following? (define_insn "*setmem_long" [(clobber (match_operand:<DBL> 0 "register_operand" "=d")) (set (mem:BLK (subreg:P (match_operand:<DBL> 3 "register_operand" "0") 0)) (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y") (subreg:P (match_dup 3) 1)] UNSPEC_REPLICATE_BYTE)) (use (match_operand:<DBL> 1 "register_operand" "d")) (clobber (reg:CC CC_REGNUM))] [ Not sure if we'd need an extra (use (match_dup 3)) any more. ] B.t.w. this is certainly wrong and cannot be generated by common code: (and:BLK (unspec:BLK [(match_operand:P 2 "shift_count_or_setmem_operand" "Y")] UNSPEC_P_TO_BLK) (match_operand 4 "const_int_operand" "n")) (This explains why the pattern would never match.) The AND should be on the filler byte instead: (unspec:BLK [(and:P (match_operand:P 2 "shift_count_or_setmem_operand" "Y") (match_operand:P 4 "const_int_operand" "n")) (subreg:P (match_dup 3) 1)] UNSPEC_REPLICATE_BYTE)) Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain ulrich.weig...@de.ibm.com