Linux kernel staging patches
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@inria.fr>
To: Alex Elder <elder@ieee.org>
Cc: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>,
	gregkh@linuxfoundation.org,  outreachy@lists.linux.dev,
	johan@kernel.org, elder@kernel.org,
	 linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev
Subject: Re: [PATCH v2] staging: greybus: use inline function for macros
Date: Wed, 22 Mar 2023 11:00:41 +0100 (CET)	[thread overview]
Message-ID: <eabacc6f-c7fb-ff33-26d1-271537fb4760@inria.fr> (raw)
In-Reply-To: <48674d8f-9753-780c-f37c-f83ea2855ae6@ieee.org>

Greg raised the question of whether the inline function is really as
efficient as a macro.

I tried the following definitions:

#define to_gbphy_dev(d) container_of(d, struct gbphy_device, dev)

static inline struct gbphy_device *extra_to_gbphy_dev(const struct device *_dev)
{
       return container_of(_dev, struct gbphy_device, dev);
}

And the following uses:

ssize_t macro_protocol_id_show(struct device *dev,
                                struct device_attribute *attr, char *buf)
{
        struct gbphy_device *gbphy_dev = to_gbphy_dev(dev);

        return sprintf(buf, "%c macro 0x%02x\n", *buf, gbphy_dev->cport_desc->protocol_id);
}
ssize_t extra_protocol_id_show(struct device *dev,
				struct device_attribute *attr, char *buf)
{
        struct gbphy_device *gbphy_dev = extra_to_gbphy_dev(dev);

        return sprintf(buf, "extra 0x%02x %c\n", gbphy_dev->cport_desc->protocol_id, *buf);
}

They are a little bit different to avoid too much compiler optimization.

After doing make drivers/staging/greybus/gbphy.s, I get similar looking
code in both cases:

Macro version:

        .type   macro_protocol_id_show, @function
macro_protocol_id_show:
        endbr64
1:      call    __fentry__
        .section __mcount_loc, "a",@progbits
        .quad 1b
        .previous
        pushq   %rbp    #
        movq    %rdx, %rbp      # tmp96, buf
        pushq   %rbx    #
# drivers/staging/greybus/gbphy.c:40: {
        movq    %rdi, %rbx      # tmp95, dev
# drivers/staging/greybus/gbphy.c:43:   return sprintf(buf, "%c macro 0x%02x\n", *buf, gbphy_dev->cport_desc->protocol_id);
        call    __sanitizer_cov_trace_pc        #
# drivers/staging/greybus/gbphy.c:43:   return sprintf(buf, "%c macro 0x%02x\n", *buf, gbphy_dev->cport_desc->protocol_id);
        movq    -32(%rbx), %rax # MEM[(struct gbphy_device *)dev_7(D) + -40B].cport_desc, MEM[(struct gbphy_device *)dev_7(D) + -40B].cport_desc
# drivers/staging/greybus/gbphy.c:43:   return sprintf(buf, "%c macro 0x%02x\n", *buf, gbphy_dev->cport_desc->protocol_id);
        movzbl  0(%rbp), %edx   # *buf_9(D), *buf_9(D)
        movq    %rbp, %rdi      # buf,
        movq    $.LC18, %rsi    #,
        movzbl  3(%rax), %ecx   # _1->protocol_id, _1->protocol_id
        call    sprintf #
# drivers/staging/greybus/gbphy.c:44: }
        movl    $13, %eax       #,
        popq    %rbx    #
        popq    %rbp    #
        jmp     __x86_return_thunk
        .size   macro_protocol_id_show, .-macro_protocol_id_show

Function version:

        .type   extra_protocol_id_show, @function
extra_protocol_id_show:
        endbr64
1:      call    __fentry__
        .section __mcount_loc, "a",@progbits
        .quad 1b
        .previous
        pushq   %rbp    #
        movq    %rdx, %rbp      # tmp96, buf
        pushq   %rbx    #
# drivers/staging/greybus/gbphy.c:47: {
        movq    %rdi, %rbx      # tmp95, dev
# drivers/staging/greybus/gbphy.c:50:   return sprintf(buf, "extra 0x%02x %c\n", gbphy_dev->cport_desc->protocol_id, *buf);
        call    __sanitizer_cov_trace_pc        #
# drivers/staging/greybus/gbphy.c:50:   return sprintf(buf, "extra 0x%02x %c\n", gbphy_dev->cport_desc->protocol_id, *buf);
        movq    -32(%rbx), %rax # MEM[(struct gbphy_device *)dev_8(D) + -40B].cport_desc, MEM[(struct gbphy_device *)dev_8(D) + -40B].cport_desc
# drivers/staging/greybus/gbphy.c:50:   return sprintf(buf, "extra 0x%02x %c\n", gbphy_dev->cport_desc->protocol_id, *buf);
        movzbl  0(%rbp), %ecx   # *buf_9(D), *buf_9(D)
        movq    %rbp, %rdi      # buf,
        movq    $.LC19, %rsi    #,
        movzbl  3(%rax), %edx   # _3->protocol_id, _3->protocol_id
        call    sprintf #
# drivers/staging/greybus/gbphy.c:51: }
        movl    $13, %eax       #,
        popq    %rbx    #
        popq    %rbp    #
        jmp     __x86_return_thunk
        .size   extra_protocol_id_show, .-extra_protocol_id_show

Both seem to access the memory directly.  Maybe the example is too simple,
and the compiler is more likely to optimize aggressively?

julia

  reply	other threads:[~2023-03-22 10:00 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-21 18:34 [PATCH v2] staging: greybus: use inline function for macros Menna Mahmoud
2023-03-21 18:50 ` Alex Elder
2023-03-21 20:43   ` Julia Lawall
2023-03-21 21:09     ` Alex Elder
2023-03-21 21:29       ` Julia Lawall
2023-03-21 22:42         ` Alex Elder
2023-03-22 10:00           ` Julia Lawall [this message]
2023-03-22 12:39             ` Alex Elder
2023-03-23  4:58             ` Greg KH
2023-03-23  5:05               ` Deepak R Varma
2023-03-23  5:22                 ` Greg KH
2023-03-23 19:46                   ` Deepak R Varma
2023-03-23  9:52               ` Julia Lawall
2023-03-25  8:49                 ` Greg KH
2023-03-25  9:28                   ` Julia Lawall
2023-03-22  9:13 ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2023-03-19 20:49 Menna Mahmoud
2023-03-19 20:56 ` Julia Lawall

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=eabacc6f-c7fb-ff33-26d1-271537fb4760@inria.fr \
    --to=julia.lawall@inria.fr \
    --cc=elder@ieee.org \
    --cc=elder@kernel.org \
    --cc=eng.mennamahmoud.mm@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=outreachy@lists.linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox