Re: [PATCH 20/24] tcg/i386: Add cf parameter to tcg_out_cmp

[Richard Henderson <richard.henderson@linaro.org>]

On 8/11/23 03:45, Peter Maydell wrote:
> On Fri, 11 Aug 2023 at 11:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Tue, 8 Aug 2023 at 04:13, Richard Henderson
>> <richard.henderson@linaro.org> wrote:
>>>
>>> Add the parameter to avoid TEST and pass along to tgen_arithi.
>>> All current users pass false.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   tcg/i386/tcg-target.c.inc | 16 ++++++++--------
>>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
>>> index b88fc14afd..56549ff2a0 100644
>>> --- a/tcg/i386/tcg-target.c.inc
>>> +++ b/tcg/i386/tcg-target.c.inc
>>> @@ -1418,15 +1418,15 @@ static void tcg_out_jxx(TCGContext *s, int opc, TCGLabel *l, bool small)
>>>       }
>>>   }
>>>
>>> -static void tcg_out_cmp(TCGContext *s, TCGArg arg1, TCGArg arg2,
>>> -                        int const_arg2, int rexw)
>>> +static void tcg_out_cmp(TCGContext *s, int rexw, TCGArg arg1, TCGArg arg2,
>>> +                        int const_arg2, bool cf)
>>>   {
>>>       if (const_arg2) {
>>> -        if (arg2 == 0) {
>>> +        if (arg2 == 0 && !cf) {
>>>               /* test r, r */
>>>               tcg_out_modrm(s, OPC_TESTL + rexw, arg1, arg1);
>>>           } else {
>>> -            tgen_arithi(s, ARITH_CMP + rexw, arg1, arg2, 0);
>>> +            tgen_arithi(s, ARITH_CMP + rexw, arg1, arg2, cf);
>>>           }
>>>       } else {
>>>           tgen_arithr(s, ARITH_CMP + rexw, arg1, arg2);
>>
>> I don't really understand the motivation here.
>> Why are some uses of this function fine with using the TEST
>> insn, but some must avoid it? What does 'cf' stand for?
>> A comment would help here if there isn't a clearer argument
>> name available...
> 
> Looking at the following patch suggests perhaps:
> 
> /**
>   * tcg_out_cmp: Emit a compare, setting the X, Y, Z flags accordingly.
>   * @need_cf : true if the comparison must also set CF
>   */
> 
> (fill in which XYZ flags you can rely on even if need_cf is false)

I can add that, yes.

Basically, test sets SZ flags, where cmp sets SZCO.  I want to add an optimizaton using C, 
so "cmp 0,x" should not be silently replaced by "test x,x".


r~