This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: S/390: Fix warnings in "*setmem_long..." patterns.
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: krebbel at linux dot vnet dot ibm dot com (Andreas Krebbel)
- Cc: gcc-patches at gcc dot gnu dot org, Ulrich dot Weigand at de dot ibm dot com (Ulrich Weigand)
- Date: Mon, 30 Nov 2015 18:11:33 +0100 (CET)
- Subject: Re: S/390: Fix warnings in "*setmem_long..." patterns.
- Authentication-results: sourceware.org; auth=none
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.Weigand@de.ibm.com