* [PATCH] Hexagon (target/hexagon) TCG generation cleanup
@ 2021-03-15 4:54 Taylor Simpson
2021-03-15 13:40 ` Richard Henderson
0 siblings, 1 reply; 3+ messages in thread
From: Taylor Simpson @ 2021-03-15 4:54 UTC (permalink / raw)
To: qemu-devel; +Cc: philmd, tsimpson, richard.henderson
Simplify TCG generation of hex_reg_written
Address feedback from Richard Henderson <<richard.henderson@linaro.org>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
target/hexagon/genptr.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 7481f4c..0ad63fe 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -43,9 +43,15 @@ static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot)
tcg_gen_movcond_tl(TCG_COND_EQ, hex_new_value[rnum], slot_mask, zero,
val, hex_new_value[rnum]);
#if HEX_DEBUG
- /* Do this so HELPER(debug_commit_end) will know */
- tcg_gen_movcond_tl(TCG_COND_EQ, hex_reg_written[rnum], slot_mask, zero,
- one, hex_reg_written[rnum]);
+ /*
+ * Do this so HELPER(debug_commit_end) will know
+ *
+ * Note that slot_mask indicates the value is not written
+ * (i.e., slot was cancelled), so we negate the value before
+ * or'ing with hex_reg_written[rnum].
+ */
+ tcg_gen_xori_tl(slot_mask, slot_mask, 1 << slot);
+ tcg_gen_or_tl(hex_reg_written[rnum], hex_reg_written[rnum], slot_mask);
#endif
tcg_temp_free(one);
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Hexagon (target/hexagon) TCG generation cleanup
2021-03-15 4:54 [PATCH] Hexagon (target/hexagon) TCG generation cleanup Taylor Simpson
@ 2021-03-15 13:40 ` Richard Henderson
2021-03-15 21:45 ` Taylor Simpson
0 siblings, 1 reply; 3+ messages in thread
From: Richard Henderson @ 2021-03-15 13:40 UTC (permalink / raw)
To: Taylor Simpson, qemu-devel; +Cc: philmd
On 3/14/21 10:54 PM, Taylor Simpson wrote:
> Simplify TCG generation of hex_reg_written
>
> Address feedback from Richard Henderson <<richard.henderson@linaro.org>
>
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> ---
> target/hexagon/genptr.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> index 7481f4c..0ad63fe 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -43,9 +43,15 @@ static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot)
> tcg_gen_movcond_tl(TCG_COND_EQ, hex_new_value[rnum], slot_mask, zero,
> val, hex_new_value[rnum]);
> #if HEX_DEBUG
> - /* Do this so HELPER(debug_commit_end) will know */
> - tcg_gen_movcond_tl(TCG_COND_EQ, hex_reg_written[rnum], slot_mask, zero,
> - one, hex_reg_written[rnum]);
> + /*
> + * Do this so HELPER(debug_commit_end) will know
> + *
> + * Note that slot_mask indicates the value is not written
> + * (i.e., slot was cancelled), so we negate the value before
> + * or'ing with hex_reg_written[rnum].
> + */
> + tcg_gen_xori_tl(slot_mask, slot_mask, 1 << slot);
> + tcg_gen_or_tl(hex_reg_written[rnum], hex_reg_written[rnum], slot_mask);
reg_written appears to be a boolean, not a mask of any kind.
I think you want
tcg_gen_setcond_i32(TCG_COND_EQ, slot_mask, slot_mask, zero);
and not the xor.
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] Hexagon (target/hexagon) TCG generation cleanup
2021-03-15 13:40 ` Richard Henderson
@ 2021-03-15 21:45 ` Taylor Simpson
0 siblings, 0 replies; 3+ messages in thread
From: Taylor Simpson @ 2021-03-15 21:45 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: philmd
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Monday, March 15, 2021 8:40 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@redhat.com
> Subject: Re: [PATCH] Hexagon (target/hexagon) TCG generation cleanup
>
> On 3/14/21 10:54 PM, Taylor Simpson wrote:
> > Simplify TCG generation of hex_reg_written
> >
> > Address feedback from Richard Henderson
> <<richard.henderson@linaro.org>
> >
> > Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> > ---
> > target/hexagon/genptr.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> > index 7481f4c..0ad63fe 100644
> > --- a/target/hexagon/genptr.c
> > +++ b/target/hexagon/genptr.c
> > @@ -43,9 +43,15 @@ static inline void gen_log_predicated_reg_write(int
> rnum, TCGv val, int slot)
> > tcg_gen_movcond_tl(TCG_COND_EQ, hex_new_value[rnum],
> slot_mask, zero,
> > val, hex_new_value[rnum]);
> > #if HEX_DEBUG
> > - /* Do this so HELPER(debug_commit_end) will know */
> > - tcg_gen_movcond_tl(TCG_COND_EQ, hex_reg_written[rnum],
> slot_mask, zero,
> > - one, hex_reg_written[rnum]);
> > + /*
> > + * Do this so HELPER(debug_commit_end) will know
> > + *
> > + * Note that slot_mask indicates the value is not written
> > + * (i.e., slot was cancelled), so we negate the value before
> > + * or'ing with hex_reg_written[rnum].
> > + */
> > + tcg_gen_xori_tl(slot_mask, slot_mask, 1 << slot);
> > + tcg_gen_or_tl(hex_reg_written[rnum], hex_reg_written[rnum],
> slot_mask);
>
> reg_written appears to be a boolean, not a mask of any kind.
> I think you want
>
> tcg_gen_setcond_i32(TCG_COND_EQ, slot_mask, slot_mask, zero);
>
> and not the xor.
I'm treating it as a zero/non-zero value. The change works because the usage in op_helper.c is
if (env->reg_written[i]) ...
I'll change the xor to setcond to make it more clear.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-15 21:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 4:54 [PATCH] Hexagon (target/hexagon) TCG generation cleanup Taylor Simpson
2021-03-15 13:40 ` Richard Henderson
2021-03-15 21:45 ` Taylor Simpson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.