On 05/19/2016 05:18 AM, Dominik Vogt wrote:
On Mon, May 16, 2016 at 01:09:36PM -0600, Jeff Law wrote:
> On 05/11/2016 02:52 AM, Dominik Vogt wrote:
> >On Wed, May 11, 2016 at 10:40:11AM +0200, Bernd Schmidt wrote:
> >That's what I mentioned somewhere during the discussion. The s390
> >backend just uses COSTS_N_INSNS(1) for AND as well as ZERO_EXTEND,
> >so this won't ever trigger. I just left the rtx_cost call in the
> >patch for further discussion as Jeff said he liked the approach.
> >We don't need it to achieve the behaviour we want for s390.
> I liked it, just based on the general theory that we should be
> comparing costs of a transform to the original much more often than
> we currently do.
>
> If Bernd prefers it gone and you don't need it to achieve your
> goals, then I won't object to the costing stuff going away.
All right, third version attached, without the rtx_vost call;
bootstrapped and regression tested on s390, s390x, x86_64.
On Wed, Apr 27, 2016 at 09:20:05AM +0100, Dominik Vogt wrote:
> The attached patch is a result of discussing an S/390 issue with
> "and with complement" in some cases.
>
> https://gcc.gnu.org/ml/gcc/2016-03/msg00163.html
> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01586.html
>
> Combine would merge a ZERO_EXTEND and a SET taking the known zero
> bits into account, resulting in an AND. Later on,
> make_compound_operation() fails to replace that with a ZERO_EXTEND
> which we get for free on S/390 but leaves the AND, eventually
> resulting in two consecutive AND instructions.
>
> The current code in make_compound_operation() that detects
> opportunities for ZERO_EXTEND does not work here because it does
> not take the known zero bits into account:
>
> /* If the constant is one less than a power of two, this might be
> representable by an extraction even if no shift is present.
> If it doesn't end up being a ZERO_EXTEND, we will ignore it unless
> we are in a COMPARE. */
> else if ((i = exact_log2 (UINTVAL (XEXP (x, 1)) + 1)) >= 0)
> new_rtx = make_extraction (mode,
> make_compound_operation (XEXP (x, 0),
> next_code),
> 0, NULL_RTX, i, 1, 0, in_code == COMPARE);
>
> An attempt to use the zero bits in the above conditions resulted
> in many situations that generated worse code, so the patch tries
> to fix this in a more conservative way. While the effect is
> completely positive on S/390, this will very likely have
> unforeseeable consequences on other targets.
>
> Bootstrapped and regression tested on s390 and s390x only at the
> moment.
Ciao
Dominik ^_^ ^_^
-- Dominik Vogt IBM Germany
0001-v3-ChangeLog
gcc/ChangeLog
* combine.c (make_compound_operation): Take known zero bits into
account when checking for possible zero_extend.
gcc/testsuite/ChangeLog
* gcc.dg/zero_bits_compound-1.c: New test.
* gcc.dg/zero_bits_compound-2.c: New test.
I'm a little worried about the tests. They check for lp64, but the
tests actually need a stronger set of conditions to pass.
I'm thinking that the tests ought to be opt-in as I don't think we have
a set of effective-target tests we can use.
So OK with the tests restricted to the targets where you've verified
they work.
Jeff
