Use of conditionals with omitted operands in amdgpu (x? : y) (was: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3))

Paul Menzel pmenzel at molgen.mpg.de
Tue Oct 19 07:44:20 UTC 2021


Dear Luben,


Am 19.10.21 um 06:50 schrieb Luben Tuikov:
> On 2021-10-19 00:38, Lazar, Lijo wrote:
>>
>> On 10/19/2021 9:45 AM, Luben Tuikov wrote:
>>> On 2021-10-18 23:38, Lazar, Lijo wrote:
>>>> On 10/19/2021 5:19 AM, Luben Tuikov wrote:

[…]

>>>>> -			if (ret)
>>>>> -				goto print_clk_out;
>>>>> +			freq_value[1] = curr_value ?: freq_value[0];
>>>> Omitting second expression is not standard C -
>>>> https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html
>>> Lijo just clarified to me that:
>>>
>>>> well, i had to look up as I haven't seen it before
>>> I hope the following should make it clear about its usage:
>>>
>>> $cd linux/
>>> $find . -name "*.[ch]" -exec grep -E "\?:" \{\} \+ | wc -l
>>> 1042
>>> $_

     $ git grep -E "\?:" -- '*amdgpu*.[ch]'
     drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c: * Solution?:

So for the AMDGPU subsystem, as the only result is a comment, currently, 
conditionals with omitted operands are not used. So, it’s a valid 
question, if the use should be introduced into the subsystem.

The GCC documentation also states:

> In this simple case, the ability to omit the middle operand is not
> especially useful. When it becomes useful is when the first operand
> does, or may (if it is a macro argument), contain a side effect. Then
> repeating the operand in the middle would perform the side effect
> twice. Omitting the middle operand uses the value already computed
> without the undesirable effects of recomputing it.

So, in your case, there are no side effect, if I am not mistaken.

I do not care, if the extension is going to be used or not. The 
maintainers might want to officially confirm the use in the subsystem, 
as using these extensions is surprising for some C developers not 
knowing the GNU extensions.

>> Thanks Luben!
> 
> You're welcome. I'm glad you're learning new things from my patches.
> Would've been easier if you'd just said in your email that you've
> never seen this ternary conditional shortcut before and that you've
> just learned of it from my patch. (Or not post anything at all in
> this very case and get in touch with me privately via email or
> Teams--I would've gladly clarified it there.)

In my opinion, asking this on the list is perfectly valid, as other 
readers, might have the same question. But being more elaborate to avoid 
misunderstandings is always a good thing.

> I hope the find+egrep above is also edifying, so you can use it in
> the future in your learning process.

I hope, you like my solution without using find. ;-)


Kind regards,

Paul


More information about the amd-gfx mailing list