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

[Danilo Krummrich]

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

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 the
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-cases
where the offset is not a "true" constant. (And I would guess that this also
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 = 0xff;
	}

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