Re: [PATCH-for-11.1 v6 3/6] monitor: Have MonitorDef::get_value() always return int64_t type

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 3/24/26 10:51 PM, Markus Armbruster wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> On 3/24/26 7:42 AM, Markus Armbruster wrote:
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>
>>>> On Tue, 24 Mar 2026 at 12:57, Markus Armbruster <armbru@redhat.com> wrote:
>>>>>
>>>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>>>
>>>>>> Simplify MonitorDef::get_value() handler by having it always
>>>>>> return a int64_t type. Truncate to 32-bit in the single caller.
>>>>>>
>>>>>> Note, this handler is only implemented once for the x86 targets.
>>>>
>>>>>> @@ -78,7 +80,8 @@ int get_monitor_def(Monitor *mon, int64_t *pval, const char *name)
>>>>>>        for(; md->name != NULL; md++) {
>>>>>>            if (hmp_compare_cmd(name, md->name)) {
>>>>>>                if (md->get_value) {
>>>>>> -                *pval = md->get_value(mon, md, md->offset);
>>>>>> +                int64_t val = md->get_value(mon, md, md->offset);
>>>>>> +                *pval = target_long_bits() == 32 ? (int32_t)val : val;
>>>>>
>>>>> This assumes target_long_bits() returns either 32 or 64, doesn't it?
>>>>>
>>>>> Is this true today?
>>>>
>>>> It's certainly true today, and we insist on that: exec/target_long.h
>>>> handles TARGET_LONG_SIZE being 4 or 8 and will #error on anything else.
>>>
>>> Good.
>>>
>>>> What other values do you expect it could have ?
>>>
>>> There might be a need for 128 in the future.  Not an easy change to
>>> make.
>>>
>>
>> The day this will happen, there will be more places to fix than the
>> current patch, as it's assumed everywhere else that target_long_bits or
>> TARGET_LONG_BITS is 32 or 64 only. So I would not worry too much about
>> it at the moment.
> 
> Yes, not an easy change to make.
> 
> I guess my comment came out of a feeling of unease about
> 
>      target_long_bits() == 32 ? [32 bit code] : [64 bit code]
> 
> I feel there's a bit of friction between abstract requirements and the
> concrete software interface.  In the abstract, we need to ask a yes/no
> question here.  The concrete interface provides a function that returns
> 32/64, which works, but isn't obvious from the type.  I guess it's
> obvious enough for anybody working in this area of the code.
> 
> If truncating to the target's actual width is common, consider providing
> function for that.
>

I think the initial reason for this to be written this way was to allow 
arithmetic expressions (and masks) with this value directly, instead of 
having ternary expression everywhere:
TARGET_IS_64_BITS ? _ : _

> Feel free to ignore me :)
> 
> [...]
>