Re: [PATCH 04/11] target/hexagon: guard writes to unimplemented guest registers

[Brian Cain <brian.cain@oss.qualcomm.com>]

On 8/20/2026 1:44 PM, Pierrick Bouvier wrote:
> On 8/18/2026 6:31 PM, Brian Cain wrote:
>> Gate guest-register writes on greg_writable() in the generated code so
>> writes to gregs above G3 are dropped instead of dereferencing an
>> unallocated TCG temp.
>>
>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>> ---
>>   target/hexagon/translate.h   |  7 ++++---
>>   target/hexagon/hex_common.py | 18 +++++++++++++++++-
>>   2 files changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
>> index 00de2b0d2ec..05425d92b29 100644
>> --- a/target/hexagon/translate.h
>> +++ b/target/hexagon/translate.h
>> @@ -97,9 +97,10 @@ bool is_gather_store_insn(DisasContext *ctx);
>>   #ifndef CONFIG_USER_ONLY
>>   static inline void ctx_log_greg_write(DisasContext *ctx, int rnum)
>>   {
>> -    assert(rnum <= HEX_GREG_G3);
>> -    ctx->greg_log[ctx->greg_log_idx] = rnum;
>> -    ctx->greg_log_idx++;
>> +    if (rnum <= HEX_GREG_G3) {
>> +        ctx->greg_log[ctx->greg_log_idx] = rnum;
>> +        ctx->greg_log_idx++;
>> +    }
>>   }
>>
> I'm not sure how this change is related to what is given in written
> message. The functional change here is that we'll ignore write, instead
> of asserting. Is that still expected to have a write on a wrong register
> anyway?
>
> If not, maybe we should at least log a guest_error instead of silently
> ignoring it.

Indeed there is an unimp logged, by greg_writable().  The fact that we 
have an analysis phase, reviewing the instructions in a packet and a 
subsequent generation phase obscures things a bit and the commit message 
can be improved.

I'll work on making things clearer/more explicit here in v2.

>>   static inline void ctx_log_greg_write_pair(DisasContext *ctx, int rnum)
>> diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
>> index e33d43e3ce0..c180c19b092 100755
>> --- a/target/hexagon/hex_common.py
>> +++ b/target/hexagon/hex_common.py
>> @@ -1097,11 +1097,24 @@ def analyze_write(self, f, tag, regno):
>>           """))
>>   
>>   class GuestRegister(Register):
>> -    pass
>> +    def gen_check_impl(self, f, regno):
>> +        if self.is_written():
>> +            f.write(code_fmt(f"""\
>> +                if (!greg_writable(insn->regno[{regno}],
>> +                    {str(self.is_pair()).lower()})) {{
>> +                    return;
>> +                }}
>> +            """))
>> +        else:
>> +            f.write(code_fmt(f"""\
>> +                check_greg_impl(insn->regno[{regno}],
>> +                                {str(self.is_pair()).lower()});
>> +            """))
>>   
>>   class GuestDest(GuestRegister, Single, Dest):
>>       def decl_tcg(self, f, tag, regno):
>>           self.decl_reg_num(f, regno)
>> +        self.gen_check_impl(f, regno)
>>           f.write(code_fmt(f"""\
>>               TCGv_i32 {self.reg_tcg()} = tcg_temp_new_i32();
>>           """))
>> @@ -1121,6 +1134,7 @@ def decl_reg_num(self, f, regno):
>>           """))
>>       def decl_tcg(self, f, tag, regno):
>>           self.decl_reg_num(f, regno)
>> +        self.gen_check_impl(f, regno)
>>           f.write(code_fmt(f"""\
>>               TCGv_i32 {self.reg_tcg()} = tcg_temp_new_i32();
>>               gen_read_greg({self.reg_tcg()}, {self.reg_num});
>> @@ -1131,6 +1145,7 @@ def analyze_read(self, f, regno):
>>   class GuestPairDest(GuestRegister, Pair, Dest):
>>       def decl_tcg(self, f, tag, regno):
>>           self.decl_reg_num(f, regno)
>> +        self.gen_check_impl(f, regno)
>>           f.write(code_fmt(f"""\
>>               TCGv_i64 {self.reg_tcg()} = tcg_temp_new_i64();
>>           """))
>> @@ -1150,6 +1165,7 @@ def decl_reg_num(self, f, regno):
>>           """))
>>       def decl_tcg(self, f, tag, regno):
>>           self.decl_reg_num(f, regno)
>> +        self.gen_check_impl(f, regno)
>>           f.write(code_fmt(f"""\
>>               TCGv_i64 {self.reg_tcg()} = tcg_temp_new_i64();
>>               gen_read_greg_pair({self.reg_tcg()}, {self.reg_num});