Hello Julien, Volodymyr and Oleksandr,
On 01.09.25 19:16, Julien Grall wrote:
> Hi,
>
> On 01/09/2025 15:42, Leonid Komarianskyi wrote:
>>>> Taking into account that with CONFIG_GICV3_ESPI=n we should never have
>>>> "irq" in eSPI range, do you really need this #ifdef? I think that
>>>> ASSERT_UNREACHABLE in espi_to_desc() is sufficient guard.
>>>>
>>>> Also, IRQ line number belongs to eSPI range regardless of
>>>> CONFIG_GICV3_ESPI,
>>>> value, so in my opinion is_espi() should always return correct value
>>>> for
>>>> a given "irq".
>>>
>>> ... I agree with Volodymyr's suggestion for is_espi() to always
>>> return
>>> correct value for a given "irq".
>>>
>>>
>>
>> I will fix that in V6.
>
> I am not sure about this. If is_espi() is not returning false with
> CONFIG_GICV3_EPSI, then the compiler would not be able to optimize code
> like:
>
> if (is_espi(...)) {
> return espi_to_desc(irq);
> }
>
> return &irq_desc[irq-NR_LOCAL_IRQS];
>
> irq_to_desc() is called fairly often, so I would like to keep the code
> fairly optimized. An alternative would be to use #ifdef CONFIG_*. I
> don't like it, but it could be a compromise if Oleksandr and Volodymyr
> wants to push to remove #ifdef from CONFIG_IS_ESPI.
>
> Cheers,
>
I am just thinking about a possible compromise between writing logical
code where the function name reflects its actual functionality and
allowing for compiler optimization. Perhaps it would be better to keep
the #ifdef but rename the function to `is_valid_espi()` or something
similar.
In that case, I think there would be less confusion, as it seems
reasonable for a function with such a name to return false when
CONFIG_GICV3_ESPI=n, and also the compiler would be able to optimize the
code.
What do you think about this approach?
Best regards,
Leonid
