Re: [PATCH 03/11] gpu: nova-core: support header parsing on Turing/GA100

[Joel Fernandes <joelagnelf@nvidia.com>]

On 11/17/2025 8:12 PM, John Hubbard wrote:
> On 11/17/25 5:04 PM, Joel Fernandes wrote:
>> On Tue, Nov 18, 2025 at 12:52:47AM +0000, Timur Tabi wrote:
>> [...] 
>>>>> +
>>>>>          Ok(Self {
>>>>> -            imem_sec_load_target: FalconLoadTarget {
>>>>> -                src_start: app0.offset,
>>>>> -                dst_start: 0,
>>>>> -                len: app0.len,
>>>>> +            imem_sec_load_target: if chipset > Chipset::GA100 {
>>>>> +                FalconLoadTarget {
>>>>> +                    src_start: app0.offset,
>>>>> +                    dst_start: 0,
>>>>> +                    len: app0.len,
>>>>> +                }
>>>>> +            } else {
>>>>> +                FalconLoadTarget {
>>>>> +                    src_start: load_hdr.os_code_size,
>>>>> +                    dst_start: app0.offset,
>>>>> +                    len: app0.len,
>>>>> +                }
>>>>
>>>> Can write more succinctly:
>>>>
>>>>   imem_sec_load_target: FalconLoadTarget {
>>>>       src_start: match chipset > Chipset::GA100  {
>>>>           true => app0.offset,
>>>>           false => load_hdr.os_code_size,
>>>>       },
>>>>       dst_start: match chipset > Chipset::GA100 {
>>>>           true => 0,
>>>>           false => app0.offset,
>>>>       
>>>>       len: app0.len,                                                                            
>>>>   },
>>>
>>> Do we really want to use "match" instead of "if", just because we don't need "else"?
>>
>> I don't care about the if/else as much as I care about the opportunity to
>> just specify FalconLoadTarget once instead twice. I think the match here is cleaner for this
>> snippet, but I am Ok with the if/else as well.
>>
>> Something like:
>>    imem_sec_load_target: FalconLoadTarget {
>>        src_start: if chipset > Chipset::GA100  {
>>           app0.offset
>>       } else {
>>            load_hdr.os_code_size
>>       },
>>
>> That would be one more line of code, but pretty much the same.
>>
> 
> You know, this latest snippet looks a little better. The pattern of 
> 
>     match a > b {
> 	true => foo,
>         false => bar,
>     }
> 
> is actually not as nice as an if-else, because, well, anytime you
> explicitly compare against true and false, you are likely doing 
> something that the language (any language) has a construct for.
> 
> And in this case, it appears to be "if/else". :)

Yeah, true. As I mentioned I was more referring to the `FalconLoadTarget`
definition. :) I prefer if it is constructed once and then fields conditionally
constructed, if possible.

thanks,

 - Joel