Re: [PATCH ix86] Fix rtx_costs for flag-setting adds
On 11/22/19 6:05 PM, Uros Bizjak wrote:
> Indeed, this is a different case, an overflow test that results in one
> CMP insn. I think, we should check if the second operand is either 0
> (then proceed as it is now), or if the second operand equals first
> operand of PLUS insn, then we actually emit CMP insn (please see
> PR30315).
Here's what I committed - preapproved by Uros off-list (I forgot
Reply-All again).
Bernd
Index: gcc/ChangeLog
===
--- gcc/ChangeLog (revision 278653)
+++ gcc/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2019-11-24 Bernd Schmidt
+
+ * config/i386/i386.c (ix86_rtx_costs): Handle care of a PLUS in a
+ COMPARE, representing an overflow detection.
+
2019-11-23 Jan Hubicka
* cif-code.def (MAX_INLINE_INSNS_SINGLE_O2_LIMIT): Remove.
Index: gcc/config/i386/i386.c
===
--- gcc/config/i386/i386.c (revision 278653)
+++ gcc/config/i386/i386.c (working copy)
@@ -19501,6 +19501,15 @@ ix86_rtx_costs (rtx x, machine_mode mode
return true;
}
+ if (GET_CODE (XEXP (x, 0)) == PLUS
+ && rtx_equal_p (XEXP (XEXP (x, 0), 0), XEXP (x, 1)))
+ {
+ /* This is an overflow detection, count it as a normal compare. */
+ *total = rtx_cost (XEXP (x, 0), GET_MODE (XEXP (x, 0)),
+ COMPARE, 0, speed);
+ return true;
+ }
+
/* The embedded comparison operand is completely free. */
if (!general_operand (XEXP (x, 0), GET_MODE (XEXP (x, 0)))
&& XEXP (x, 1) == const0_rtx)
Re: [PATCH ix86] Fix rtx_costs for flag-setting adds
On Fri, Nov 22, 2019 at 5:39 PM Bernd Schmidt wrote:
>
> On 11/22/19 3:04 PM, Uros Bizjak wrote:
> > On Fri, Nov 22, 2019 at 1:58 PM Bernd Schmidt
> > wrote:
> >>
> >> A patch I posted recently fixes combine to take costs of JUMP_INSNs into
> >> account. That causes the pr30315 test to fail with -m32, since the cost
> >> of an add that sets the flags is estimated too high.
> >>
> >> The following seems to fix it. Bootstrapped and tested on x86_64-linux,
> >> ok?
> >
> > I think that the intention of the code below "The embedded comparison
> > operand is completely free." comment is to handle this case. It looks
> > that it should return the cost of the inside operation of COMPARE rtx.
>
> There seem to be two problems with that. We're dealing with patterns
> such as:
>
> (set (reg:CCC 17 flags)
> (compare:CCC (plus:SI (mem/c:SI (reg/f:SI 16 argp) [2 a+0 S4
> A32])
> (reg/v:SI 87 [ b ]))
> (mem/c:SI (reg/f:SI 16 argp) [2 a+0 S4 A32])))
Indeed, this is a different case, an overflow test that results in one
CMP insn. I think, we should check if the second operand is either 0
(then proceed as it is now), or if the second operand equals first
operand of PLUS insn, then we actually emit CMP insn (please see
PR30315).
Uros.
> If I remove the test for const0_rtx, it still doesn't work - I think
> setting *total to zero is ineffective, since we'll still count the MEM
> twice.
>
> So, how about the following?
>
>
> Bernd
>
> @@ -19502,9 +19502,12 @@
> }
>
>/* The embedded comparison operand is completely free. */
> - if (!general_operand (XEXP (x, 0), GET_MODE (XEXP (x, 0)))
> - && XEXP (x, 1) == const0_rtx)
> - *total = 0;
> + if (!general_operand (XEXP (x, 0), GET_MODE (XEXP (x, 0
> + {
> + *total = rtx_cost (XEXP (x, 0), GET_MODE (XEXP (x, 0)),
> +COMPARE, 0, speed);
> + return true;
> + }
>
>return false;
>
Re: [PATCH ix86] Fix rtx_costs for flag-setting adds
On 11/22/19 3:04 PM, Uros Bizjak wrote:
> On Fri, Nov 22, 2019 at 1:58 PM Bernd Schmidt wrote:
>>
>> A patch I posted recently fixes combine to take costs of JUMP_INSNs into
>> account. That causes the pr30315 test to fail with -m32, since the cost
>> of an add that sets the flags is estimated too high.
>>
>> The following seems to fix it. Bootstrapped and tested on x86_64-linux, ok?
>
> I think that the intention of the code below "The embedded comparison
> operand is completely free." comment is to handle this case. It looks
> that it should return the cost of the inside operation of COMPARE rtx.
There seem to be two problems with that. We're dealing with patterns
such as:
(set (reg:CCC 17 flags)
(compare:CCC (plus:SI (mem/c:SI (reg/f:SI 16 argp) [2 a+0 S4
A32])
(reg/v:SI 87 [ b ]))
(mem/c:SI (reg/f:SI 16 argp) [2 a+0 S4 A32])))
If I remove the test for const0_rtx, it still doesn't work - I think
setting *total to zero is ineffective, since we'll still count the MEM
twice.
So, how about the following?
Bernd
@@ -19502,9 +19502,12 @@
}
/* The embedded comparison operand is completely free. */
- if (!general_operand (XEXP (x, 0), GET_MODE (XEXP (x, 0)))
- && XEXP (x, 1) == const0_rtx)
- *total = 0;
+ if (!general_operand (XEXP (x, 0), GET_MODE (XEXP (x, 0
+ {
+ *total = rtx_cost (XEXP (x, 0), GET_MODE (XEXP (x, 0)),
+COMPARE, 0, speed);
+ return true;
+ }
return false;
Re: [PATCH ix86] Fix rtx_costs for flag-setting adds
On Fri, Nov 22, 2019 at 1:58 PM Bernd Schmidt wrote: > > A patch I posted recently fixes combine to take costs of JUMP_INSNs into > account. That causes the pr30315 test to fail with -m32, since the cost > of an add that sets the flags is estimated too high. > > The following seems to fix it. Bootstrapped and tested on x86_64-linux, ok? I think that the intention of the code below "The embedded comparison operand is completely free." comment is to handle this case. It looks that it should return the cost of the inside operation of COMPARE rtx. Uros.
