| Message ID | 1617249213-22667-2-git-send-email-tsimpson@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Hexagon (target/hexagon) update | expand |
On 3/31/21 8:53 PM, Taylor Simpson wrote: > Simplify TCG generation of hex_reg_written > > Suggested-by: Richard Henderson<richard.henderson@linaro.org> > Signed-off-by: Taylor Simpson<tsimpson@quicinc.com> > --- > target/hexagon/genptr.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 3/31/21 8:53 PM, Taylor Simpson wrote: > Simplify TCG generation of hex_reg_written > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Taylor Simpson <tsimpson@quicinc.com> > --- > target/hexagon/genptr.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c > index 7481f4c..87f5d92 100644 > --- a/target/hexagon/genptr.c > +++ b/target/hexagon/genptr.c > @@ -35,7 +35,6 @@ static inline TCGv gen_read_preg(TCGv pred, uint8_t num) > > static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot) > { > - TCGv one = tcg_const_tl(1); > TCGv zero = tcg_const_tl(0); > TCGv slot_mask = tcg_temp_new(); > > @@ -43,12 +42,17 @@ 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 create a true/false value before > + * or'ing with hex_reg_written[rnum]. > + */ > + tcg_gen_setcond_tl(TCG_COND_EQ, slot_mask, slot_mask, zero); > + tcg_gen_or_tl(hex_reg_written[rnum], hex_reg_written[rnum], slot_mask); > #endif Having looked forward at patch 5, it appears this could be further improved by examining ctx->regs_written. r~
> -----Original Message----- > From: Richard Henderson <richard.henderson@linaro.org> > Sent: Friday, April 2, 2021 12:47 PM > To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org > Cc: philmd@redhat.com; ale@rev.ng; Brian Cain <bcain@quicinc.com> > Subject: Re: [PATCH v2 01/21] Hexagon (target/hexagon) TCG generation > cleanup > > On 3/31/21 8:53 PM, Taylor Simpson wrote: > > Simplify TCG generation of hex_reg_written > > > > Suggested-by: Richard Henderson <richard.henderson@linaro.org> > > Signed-off-by: Taylor Simpson <tsimpson@quicinc.com> > > --- > > target/hexagon/genptr.c | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c > > index 7481f4c..87f5d92 100644 > > --- a/target/hexagon/genptr.c > > +++ b/target/hexagon/genptr.c > > @@ -35,7 +35,6 @@ static inline TCGv gen_read_preg(TCGv pred, uint8_t > num) > > > > static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int > slot) > > { > > - TCGv one = tcg_const_tl(1); > > TCGv zero = tcg_const_tl(0); > > TCGv slot_mask = tcg_temp_new(); > > > > @@ -43,12 +42,17 @@ 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 create a true/false value before > > + * or'ing with hex_reg_written[rnum]. > > + */ > > + tcg_gen_setcond_tl(TCG_COND_EQ, slot_mask, slot_mask, zero); > > + tcg_gen_or_tl(hex_reg_written[rnum], hex_reg_written[rnum], > slot_mask); > > #endif > > Having looked forward at patch 5, it appears this could be further improved > by examining ctx->regs_written. It's not obvious how. This is for a predicated instruction (e.g., if (p0) r2 = add(r1, r0)), so the checks need to be made at runtime. There can be more than one predicated instruction in a packet that writes to the same register as long as only one of the predicates is true at runtime. Am I missing something? Thanks, Taylor
On 4/2/21 12:42 PM, Taylor Simpson wrote: >>> @@ -43,12 +42,17 @@ static inline void gen_log_predicated_reg_write(int ... >> Having looked forward at patch 5, it appears this could be further improved >> by examining ctx->regs_written. > > It's not obvious how. This is for a predicated instruction (e.g., if (p0) r2 = add(r1, r0)), so the checks need to be made at runtime. There can be more than one predicated instruction in a packet that writes to the same register as long as only one of the predicates is true at runtime. > > Am I missing something? Whoops, no. I misread where we were here. r~
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c index 7481f4c..87f5d92 100644 --- a/target/hexagon/genptr.c +++ b/target/hexagon/genptr.c @@ -35,7 +35,6 @@ static inline TCGv gen_read_preg(TCGv pred, uint8_t num) static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot) { - TCGv one = tcg_const_tl(1); TCGv zero = tcg_const_tl(0); TCGv slot_mask = tcg_temp_new(); @@ -43,12 +42,17 @@ 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 create a true/false value before + * or'ing with hex_reg_written[rnum]. + */ + tcg_gen_setcond_tl(TCG_COND_EQ, slot_mask, slot_mask, zero); + tcg_gen_or_tl(hex_reg_written[rnum], hex_reg_written[rnum], slot_mask); #endif - tcg_temp_free(one); tcg_temp_free(zero); tcg_temp_free(slot_mask); }
Simplify TCG generation of hex_reg_written Suggested-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com> --- target/hexagon/genptr.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)