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

On Tue Jul 28, 2026 at 9:07 PM BST, Danilo Krummrich wrote:
> On Tue Jul 28, 2026 at 9:03 PM CEST, Gary Guo wrote:
>> On Tue Jul 28, 2026 at 7:56 PM BST, Danilo Krummrich wrote:
>>> On Tue Jul 28, 2026 at 8:26 PM CEST, Gary Guo wrote:
>>>> On Tue Jul 28, 2026 at 6:01 PM BST, Danilo Krummrich wrote:
>>>>> 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!`.
>>>
>>> arg-position const generics would be great, but that's just a syntax difference
>>> and not related to supporting expressions involving const generics?
>>>
>>> In any case, I think my point about being closer to the final solution holds
>>> regardless.
>>>
>>>> Personally I think most issues with `build_assert!` can be fixed by having
>>>> lints, which is on my radar.
>>>
>>> I think it can be improved, but in the end it relies on compiler optimization
>>> that may or may not happen as expected.
>>
>> It'd be a compiler bug if const folding is expected to happen, the whole
>> BUILD_BUG_ON depends on it working.
>>
>> `build_assert!` is a useful tool that I'm not giving up. If there is a need to
>> use it, then I'd use it compared to having two variants to do the same thing
>> with different syntax.
>
> It is indeed useful and I don't want to give up on it either.
>
> But, it is more fragile than const evaluation, so we should only use it where
> const evaluation is not sufficient.
>
> In the case we are discussing the problem is that const expressions involving
> const generics are not yet supported, but in general it should be.
>
> This is different compared to e.g. read() where the syntax difference would
> actually hurt and const generics would limit flexibility, where we actually rely
> on the compiler to eliminate unreachable code paths regardless of a value being
> known at runtime only.

The offset for `subregion` is no different to the offset for `read()`. In both
cases you are supposed to only supply build-time const values; and in both cases
you can argue they are needed there due to const generics being too limited to
what they do.

We probably could even just add a method that gives you a subregion based on
`IoLoc` similar to other methods. This actually might be a good idea that I'd
pursue in next version.

Best,
Gary