On Fri, Nov 22, 2019 at 5:39 PM Bernd Schmidt <bernds_...@t-online.de> wrote: > > On 11/22/19 3:04 PM, Uros Bizjak wrote: > > On Fri, Nov 22, 2019 at 1:58 PM Bernd Schmidt <bernds_...@t-online.de> > > 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; >