Re: [PATCH] target/s390x: Fix CC set by CONVERT TO FIXED/LOGICAL

[Ulrich Weigand]

On Fri, Jun 25, 2021 at 06:19:48PM -0700, Richard Henderson wrote:
> On 6/23/21 7:50 AM, Ulrich Weigand wrote:
> >@@ -506,6 +534,7 @@ uint64_t HELPER(cgeb)(CPUS390XState *env, uint64_t v2, 
> >uint32_t m34)
> >  {
> >      int old_mode = s390_swap_bfp_rounding_mode(env, round_from_m34(m34));
> >      int64_t ret = float32_to_int64(v2, &env->fpu_status);
> >+    env->cc_op = set_cc_conv_f32(v2, &env->fpu_status);
> ...
> 
> >@@ -1875,7 +1860,7 @@ static DisasJumpType op_cgeb(DisasContext *s, DisasOps 
> >*o)
> >     }
> >     gen_helper_cgeb(o->out, cpu_env, o->in2, m34);
> >     tcg_temp_free_i32(m34);
> >-    gen_set_cc_nz_f32(s, o->in2);
> >+    set_cc_static(s);
> >     return DISAS_NEXT;
> 
> ...
> 
> >helper.h:DEF_HELPER_FLAGS_3(clgdb, TCG_CALL_NO_WG, i64, env, i64, i32)
> 
> This won't work reliably.  You're writing to a tcg global inside of
> a function that says that it won't.

I missed that, sorry.  That problem can be fixed by changing the above
line to something like:
DEF_HELPER_3(clgdb, i64, env, i64, i32)
right?

> It's probably time to take care of
> 
> >    /*
> >     * FIXME:
> >     * 1. Right now, all inexact conditions are inidicated as
> >     *    "truncated" (0) and never as "incremented" (1) in the DXC.
> >     * 2. Only traps due to invalid/divbyzero are suppressing. Other traps
> >     *    are completing, meaning the target register has to be written!
> >     *    This, however will mean that we have to write the register before
> >     *    triggering the trap - impossible right now.
> >     */
> 
> point 2, by splitting the fpu helpers.  In the first part, take care
> of the optimization and suppressed traps, and return the register
> value.  In the second part, take care of FPC write-back, completing
> traps, and return any cc value.  Which you can then assign,
> properly, to the cc_op tcg global.

I'm not sure I see how this is related to the CC problem.  Note that
the difference between suppressing and completing traps applies to
both the target register and the CC - if a suppressing trap is
triggered, both target register and CC remain unchanged, otherwise
they are changed (and both changes are visible to the trap handler).

In any case, the current implementation already has two helpers, and
I initially tried to keep that, by using a different second part to
correctly compute CC.  But this ran into the problem that I didn't
see any way to detect the fact that the conversion operation had run
into one of the special cases in the second helper, without re-doing
the whole conversion a second time.  Is there any way to pass
information between the two helpers (without running again into the
same qemu global state updating problem)?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com