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

On Wed Jul 29, 2026 at 12:24 PM CEST, Gary Guo wrote:
> 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 fo=
r the
>>>>>> purpose of working around generic_const_exprs, as it probably is a b=
it 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-posit=
ion
>>>>> const-generics which would syntactically look more similar to `build_=
assert!`.
>>>>
>>>> arg-position const generics would be great, but that's just a syntax d=
ifference
>>>> 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 h=
aving
>>>>> lints, which is on my radar.
>>>>
>>>> I think it can be improved, but in the end it relies on compiler optim=
ization
>>>> that may or may not happen as expected.
>>>
>>> It'd be a compiler bug if const folding is expected to happen, the whol=
e
>>> 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 invo=
lving
>> const generics are not yet supported, but in general it should be.
>>
>> This is different compared to e.g. read() where the syntax difference wo=
uld
>> actually hurt and const generics would limit flexibility, where we actua=
lly rely
>> on the compiler to eliminate unreachable code paths regardless of a valu=
e 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 bot=
h cases
> you can argue they are needed there due to const generics being too limit=
ed to
> what they do.

I see where you are coming from, but I see it from a different angle:

An offset given to read() can legitimately be a runtime value that happens =
to be
constrained in a way that the compiler can proof that it is within bounds (=
with
dead code elimination doing the rest). We have reasonable use-cases where t=
he
offset given to read() is not a "true" constant, and hence build_assert! is=
 the
correct tool.

subregion() is different in this regard, as we do not have reasonable use-c=
ases
where the offset is not a "true" constant. (And I would guess that this als=
o
made you choose the implementation you picked for this patch.)

It just happens that we hit a language limitation where const evaluation of=
 a
"true" constant does not work, but that does not make build time assertion =
the
correct tool, it just makes it a workaround for the existing limitation.

This is also in line with the documentation in rust/kernel/build_assert.rs.

> 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.

So, you are proposing that subregion() takes an argument of type
L: impl IoLoc<_>?

Where would this argument come from?

I assume you would need to bring back

	impl RegisterBase<Foo> for Bar {
	    const BASE: usize =3D 0xff;
	}

impl blocks? Which seems like just adding boilerplate? Or do you have somet=
hing
else in mind?