Re: [PATCH v7 05/10] rust: io: add IoLoc and IoWrite types

["Alexandre Courbot" <acourbot@nvidia.com>]

On Sun Mar 8, 2026 at 6:10 AM JST, Gary Guo wrote:
> On Sat Mar 7, 2026 at 12:05 AM GMT, Alexandre Courbot wrote:
>> On Sat Mar 7, 2026 at 12:35 AM JST, Gary Guo wrote:
>>> On Fri Mar 6, 2026 at 2:32 PM GMT, Alexandre Courbot wrote:
>>>> On Fri Mar 6, 2026 at 10:20 PM JST, Gary Guo wrote:
>>>>> I  mean not sure `at` gives me that impression at all. It would just let me know
>>>>> that I am accessing it at a different location. If you omit the `MyRegArray`
>>>>> part then there's no real indication that this is an array to me.
>>>>
>>>> `at` is a function name, we can change it - I picked it because it is
>>>> short and reasonably descriptive. The point being: we have a unique
>>>> function that indicates unambigously that we are using a location for
>>>> an array of registers.
>>>
>>> Okay, maybe `at` is not the biggest issue. I just instinctively feel the usage
>>> example being awkward.
>>>
>>> Perhaps the fact that `Reg` exist itself is awkward to me. It looks like a type
>>> that exists only for things to typecheck, and not itself represent a meaningful
>>> concept.
>>
>> After partially implementing your two-args proposal it really turns out
>> the two alternatives are very similar in essence, with most of the
>> differences being details like naming and scope.
>>
>> The one fundamental difference is that it didn't occur to me that we
>> could resolve a generic argument for a function in first position of
>> write using another argument, hence I went for:
>>
>>     bar.write(location_builder(location_info, value));
>>
>> but it is also possible to do
>>
>>     bar.write(location_builder(location_info), value);
>>
>> And that's really the main difference. All the rest is mostly details
>> about whether `location_builder` is an associated function of a type, a
>> method of a ZST, or just a module-level function. From the point of view
>> of `Io`, none of this matters as long as it gets an `IoLoc` that is
>> compatible with the value. IOW, each Io submodule can provide the
>> location builders that make sense for it.
>>
>> The `Reg` thing in my previous email was just a way to provide a scope
>> for these location builders, but in retrospect a module-level function
>> seems better to me if we want to keep things concise. So if we turn the
>> location builders into module-level methods we could have:
>>
>>     use kernel::io::register as reg;
>>
>>     bar.write(reg::at(10), SOME_ARRAY_SCRATCH::foo());
>>
>> Or does
>>
>>     bar.write(SOME_ARRAY_SCRATCH::foo(), reg::at(10));
>>
>> Flow better now?
>
> I'm still generally inclined to have location at front, as it's also going to be
> how projection syntax will work for structs:
>
>     io_write!(bar, .scratch[10], foo);
>
> or just assignment in general
>
>     my.scratch[10] = foo;

Yes, after working on the implementation I came to a similar conclusion,
mostly because the new construct reads fine in that order.

>
> I should also say that, my desire of having the explicit locaction is also
> partly driven by the desire to eventually unify projections with the register
> macro. If we would generate a struct where fields inside are registers at there
> correct offset, then `io_write!(bar, .register_name, foo)` would indeed be a
> canonical way of accessing such registers using I/O projection. There's no way
> to simplify `my_struct.foo = Foo {}` without repeating `foo` twice :)
>
> For context, Benno suggested in rust-lang zulip in a discussion about how it
> might be possible to use field projections to replace register macro in the
> future:
> https://rust-lang.zulipchat.com/#narrow/channel/522311-t-lang.2Fcustom-refs/topic/custom.20virtual.20fields/near/573703808
> Don't worry, the language features don't exist yet, there's no change needed to
> your series :)
>
> Probably I should mention this earlier so you see the aspect I'm coming from,
> but none of this is directly related to the `register!` work you're having
> today, so I was avoiding to argue with unrelated features.

That's useful context to have indeed. To be fair I also haven't done my
homework with I/O projections - I took a peek at your pointer projection
series, but not quite enough to do a proper review.

Being able to define the register space as a regular Rust construct does
look very appealing. Actually that what the register macro tries to
mimic, albeit clumsily.

Name repetition might become a concern though, for the same reason it is
one now, but if we start to use macros like `io_write!` then maybe we
can macro our way around this as well. Or, have a way to produce
`IoLoc`s from struct fields and use the current I/O methods. Anyway it's
a concern for later.

>
>>
>>>
>>>>
>>>> You seem to reject this design because the syntax isn't obvious and
>>>> natural to you at first read. We are trying to build something new, so
>>>> of course if will look a bit alien at the first encounter. That's why we
>>>> have documentation, and I think it is not very difficult to wrap your
>>>> head around it after seeing a few examples.
>>>>
>>>>>
>>>>> If `at` is only for array, how would you represent the case where the same type
>>>>> is being used in multiple registers?
>>>>
>>>> That's not something that is supported by the register macro currently
>>>> (probably not a big change, but not something I will do in this series).
>>>>
>>>> But to try and answer your question, such register types would not have
>>>> an `IoLoc` implementation of their own and would need to have their
>>>> location constructed explicitly. In accordance, the construction of
>>>> their value would not bear any location information; thus there would be
>>>> no redundancy.
>>>>
>>>> We do have a case that is pretty close to this with relative registers.
>>>> These are accessed like this (real examples):
>>>>
>>>>     bar.write(Reg::of::<Gsp>(regs::NV_PFALCON_FALCON_DMACTL::zeroed()));
>>>>
>>>> and
>>>>
>>>>     bar.write(Reg::of::<Sec2>(regs::NV_PFALCON_FALCON_DMACTL::zeroed()));
>>>
>>> Relative registers are something that on my list to eliminate and replace with
>>> projections, so I don't particular care about how it looks like.
>>
>> That's interesting, I thought register arrays would be easier to replace
>> than relative registers, I really need to take a closer look.
>
> Register array with stride might be tricky, although I have to say I haven't put
> much thought into this yet.

Looking at the Zulip discussion, and lacking most of the context, it
seems we can either put intertwined register arrays into a struct and
make an array of that, or convert them to relative registers (or
possibly add some inaccessible padding). Register arrays that are not
contiguous tend to fall into one of these categories. IOW, proper
structure definitions we might very well be able to get rid of that
stride parameter that looks a bit out of place.

>
>>
>>>
>>>>
>>>> But for register types that can be used at several arbitrary locations,
>>>> I think this would be even simpler. The different locations would just
>>>> need to implement `IoLoc<T>`, where `T` is the shared register type.
>>>> Then, considering that the two-arguments version is called `write_at`,
>>>> you can simply do:
>>>>
>>>>    bar.write_at(REG_LOCATION, reg_value);
>>>
>>> Having `write_at` as the name of the two-argument version is okay to me.
>>
>> I picked that for illustrative purposes, but `write` should be the
>> version that is going to be the most used. And if we remove the value
>> from the location builder, then the most used version is clearly going
>> to be the two arguments one.
>>
>> The one argument variant would then become a shortcut - `put` sounds
>> adequate to me, but `write_val` also works.
>
> `put` sounds good. It was also the name that Gemini suggests to me when I asked
> it to come up with a short and concise name for this :)
>
>>
>>>
>>>>
>>>> ... but this design also makes this possible:
>>>>
>>>>    bar.write((REG_LOCATION, reg_value));
>>>
>>> I considered about this, but IMO this looks awkward.
>>
>> Funny how you spell "elegant". :)
>
> It's indeed quite elegant from a FP and type theorist lens. I'm not personally
> objecting this, although I feel that this may looks strange to people with C
> background (not sure if that's actually true, though).

Fair point, but the goal is also to allow people with a C background to
build their Rust knowledge, so I think such accomodations should be done
using thoughtful documentation rather than limiting our API choices.

Not saying this is what is happening here, I haven't seen such use of
tuples be deemed as idiomatic so I was just suggesting a way to preserve
the API purity. But I'd like to keep the `IoLoc` implementation on
tuples as it can be useful for generic code (just like `impl IoLoc<T>
for ()`).