Re: [PATCH 08/10] gpu: nova-core: use projection for PFALCON and PFALCON2 registers

[Danilo Krummrich]

On Tue Jul 28, 2026 at 5:09 PM CEST, Alexandre Courbot wrote:
> Great, keeping this commit purely mechanical sounds good to me if you
> prefer it that way (although one can argue that the refactor itself is
> also mostly mechanical as the end result shouldn't be more complex).

I don't really see much value in this intermediate change; it's not really less
invasive than to do the refactor I think, so let's do that.

>>> And I suspect that once this is generalized, `Falcon` won't even need to
>>> store a reference to the `Bar0` (and potentially poke the I/O of other
>>> engines) anymore.
>>>
>>> Actually I would like to push that even further and replace the
>>> `pfalcon()` and `pfalcon2()` trait methods by associated constants used
>>> to construct the projected view in `Falcon::new`, since the projections
>>> are all constructed the same way, but doing so requires
>>> `generic_const_exprs`. :/

Yes, that'd be best.

>>> We could make it work by moving the `OFFSET` generic argument of
>>> `subregion` into a regular argument and enforcing its invariants using
>>> `build_assert!`, but that would require `subregion` to be
>>> `#[inline(always)]`. I don't know if there is another trick we can use,
>>> if not otherwise I guess the trait methods are ok, especially if they
>>> are only called once in the constructor.

I think this is fine if, as Alex says as well, we expose it as another variant
of subregion(). (In general, I'd like to avoid introducing more build_assert!()
if there's not much value though.)

In the nova-core case it would be a specific workaround for generic_const_exprs
not being stable, so a separate subregion() variant that is specifically
targeted at this seems fine.

>> This was definitely one option that I have considered, however as `NEW_SIZE`
>> still needs to be generic so we're not entirely turbofish-free. But if making
>> `offset` become an effective const parameter helps nova impl, it might make
>> sense to use `build_assert!` for this.

As mentioned I'd like to avoid additional build_assert!() if not needed, the
turbofish doesn't hurt too much here.

> Maybe we can have another variant of `subregion` that operates that way,
> but your call. I don't see any obvious flaw with the current
> implementation, it's just that the `FalconEngine` methods should not be
> useful outside of the constructor eventually, so it seems a bit
> superfluous to have them visible to the whole crate.

I may have a slight preference for a separate subregion() variant for the
purpose of working around generic_const_exprs, as it probably is a bit closer to
the final solution, but trait methods are fine with me too.