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

[Gary Guo]

On Tue Jul 28, 2026 at 6:01 PM BST, Danilo Krummrich wrote:
> 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.

I am not sure that's the final solution that I want. I want arg-position
const-generics which would syntactically look more similar to `build_assert!`.

Personally I think most issues with `build_assert!` can be fixed by having
lints, which is on my radar.

Best,
Gary