All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Menzel <pmenzel@molgen.mpg.de>
To: Luben Tuikov <luben.tuikov@amd.com>
Cc: Alex Deucher <Alexander.Deucher@amd.com>,
	Lijo Lazar <lijo.lazar@amd.com>,
	amd-gfx@lists.freedesktop.org
Subject: 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))
Date: Tue, 19 Oct 2021 09:44:20 +0200	[thread overview]
Message-ID: <137ec58c-a8e9-19d4-c49b-19b6e0aad144@molgen.mpg.de> (raw)
In-Reply-To: <65580f09-1ad8-07ba-b392-dc4aad2464ee@amd.com>

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

  reply	other threads:[~2021-10-19  7:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-18 23:49 [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Luben Tuikov
2021-10-18 23:49 ` [PATCH 1/5] drm/amd/pm: Rename a couple of functions (v3) Luben Tuikov
2021-10-18 23:49 ` [PATCH 2/5] drm/amd/pm: Rename cur_value to curr_value Luben Tuikov
2021-10-18 23:49 ` [PATCH 3/5] drm/amd/pm: Rename freq_values --> freq_value Luben Tuikov
2021-10-18 23:49 ` [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3) Luben Tuikov
2021-10-19  3:38   ` Lazar, Lijo
2021-10-19  4:15     ` Luben Tuikov
2021-10-19  4:38       ` Lazar, Lijo
2021-10-19  4:50         ` Luben Tuikov
2021-10-19  7:44           ` Paul Menzel [this message]
2021-10-19  8:07             ` 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)) Luben Tuikov
2021-10-18 23:49 ` [PATCH 5/5] dpm/amd/pm: Navi10: Remove 0 MHz as a current clock frequency (v3) Luben Tuikov
2021-10-19  7:23 ` [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Paul Menzel
2021-10-19  7:43   ` Luben Tuikov
2021-10-19  7:54     ` Paul Menzel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=137ec58c-a8e9-19d4-c49b-19b6e0aad144@molgen.mpg.de \
    --to=pmenzel@molgen.mpg.de \
    --cc=Alexander.Deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=lijo.lazar@amd.com \
    --cc=luben.tuikov@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.