Re: [PATCH 1/5] gpu: nova-core: add Tlv.get_u64

["Eliot Courtney" <ecourtney@nvidia.com>]

On Wed Nov 11, 2026 at 12:45 PM JST, John Hubbard wrote:
> On 11/10/26 6:59 PM, Timur Tabi wrote:
>> On Tue, 2026-11-10 at 16:20 -0800, John Hubbard wrote:
>>> On 11/10/26 3:10 PM, Timur Tabi wrote:
>>> ...
>>> ...but then first_chunk() checks the length again, and also returns
>>> an Option that is never None.
>> 
>> In an optimized build, that check is not actually done.  The only checks are for the match
>> statement.
>
> Nice. OK.
>
> ...
>>> Instead, we could use try_from() to check the length and convert
>>> the slice, in one step. Your TlvBlockHeader::parse() already uses
>>> that approach, and I think it would also work here.
>>>
>>> Something like this:
>>>
>>>     pub(crate) fn get_u64(&self, tag: &[u8; 4]) -> Result<u64> {
>>>         let value = self.get_bytes(tag)?;
>>>
>>>         if let Ok(bytes) = <[u8; 4]>::try_from(value) {
>>>             Ok(u64::from(u32::from_le_bytes(bytes)))
>>>         } else if let Ok(bytes) = <[u8; 8]>::try_from(value) {
>>>             Ok(u64::from_le_bytes(bytes))
>>>         } else {
>>>             Err(EINVAL)
>>>         }
>>>     }
>> 
>> I'm more than happy to change it to whatever other people want.  I went through several
>> iterations of this code, working with Cursor to evaluate each one.  IMHO, they are all ugly and
>> hard to read, because of the from_le_bytes() constraint that I mentioned in the comment.
>
> The reason I responded was that I think the try_from() is nicer
> on-screen. I hadn't gone all the way to the asm code.
>
> thanks,

I prefer John's version which looks a bit more readable to me. It's also
possible to write it like this to avoid writing the array types,
although I don't think it's necessarily better:

```
let value = self.get_bytes(tag)?;

if let Ok(value) = value.try_into().map(u32::from_le_bytes) {
    Ok(u64::from(value))
} else if let Ok(value) = value.try_into().map(u64::from_le_bytes) {
    Ok(value)
} else {
    Err(EINVAL)
}
```