From: Nicholas Mc Guire <der.herr@hofr.at>
To: Joe Perches <joe@perches.com>
Cc: Nicholas Mc Guire <hofrat@osadl.org>,
Michal Marek <mmarek@suse.cz>,
Masahiro Yamada <yamada.m@jp.panasonic.com>,
Sam Ravnborg <sam@ravnborg.org>,
Thomas Gleixner <tglx@linutronix.de>,
"H. Peter Alvin" <hpa@zytor.com>,
John Stultz <john.stultz@linaro.org>,
Andrew Hunter <ahh@google.com>, Paul Turner <pjt@google.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] time: allow gcc to fold constants when using msecs_to_jiffies
Date: Mon, 6 Apr 2015 08:40:05 +0200 [thread overview]
Message-ID: <20150406064005.GA32350@opentech.at> (raw)
In-Reply-To: <1428294823.2775.94.camel@perches.com>
On Sun, 05 Apr 2015, Joe Perches wrote:
> On Mon, 2015-04-06 at 06:26 +0200, Nicholas Mc Guire wrote:
> > On Sun, 05 Apr 2015, Joe Perches wrote:
> > > Try it and look at the generated .lst files with and
> > > without the patch I sent.
> []
> > from all that I understood it should
> > be doable both as macro and inline.
>
> I think it _should_ be doable too but I also think
> the only reason gcc doesn't optimize the inline
> is because gcc's optimizer isn't good enough yet.
>
"unfortunately" I can't blame it on gcc - here is the initial toy-case
- test.c and either testi.h or testm.h included
- m = TIMEOUT or m = atoi(argv[1]);
both in the inline and the macro case gcc reduced the code to a single
load mediate or register instruction for the constant - so the optimizer
is doing its job.
test.c:
#include <stdio.h>
#define HZ 100
#define MSECS_PER_SEC 1000
#define TIMEOUT 100
#include "testi.h" /* inline msecs_to_jiffies */
//#include "testm.h" /* macro versions */
int main(int argc, char **argv) {
//int m = atoi(argv[0]); /* non-const */
int m = TIMEOUT; /* const */
printf("%lu\n",msecs_to_jiffies(m));
return 0;
}
testm.h:
#define msecs_to_jiffies(m) \
(__builtin_constant_p (m) \
? ((m) * HZ / MSECS_PER_SEC ) : __msecs_to_jiffies(m))
unsigned long __msecs_to_jiffies(int m)
{
return m * HZ / MSECS_PER_SEC ;
}
first case with a non-const
main:
.LFB12:
.cfi_startproc
subq $8, %rsp #,
.cfi_def_cfa_offset 16
movq 8(%rsi), %rdi # MEM[(char * *)argv_2(D) + 8B], MEM[(char * *)argv_2(D) + 8B]
xorl %eax, %eax #
call atoi #
movl $1717986919, %edx #, tmp69
movl %eax, %ecx #, m
movl $.LC0, %edi #,
imull %edx # tmp69
sarl $31, %ecx #, tmp71
xorl %eax, %eax #
sarl $2, %edx #, tmp67
subl %ecx, %edx # tmp71, tmp67
movslq %edx, %rsi # tmp67, tmp72
call printf #
o
second with a constant:
main:
.LFB12:
.cfi_startproc
subq $8, %rsp #,
.cfi_def_cfa_offset 16
movl $10, %esi #,
movl $.LC0, %edi #,
xorl %eax, %eax #
call printf #
inline:
-------
testi.h:
static inline unsigned long __msecs_to_jiffies(int m)
{
return m * HZ / MSECS_PER_SEC;
}
static inline unsigned long msecs_to_jiffies(int m)
{
return __builtin_constant_p (m) ?
(m) * HZ / MSECS_PER_SEC : __msecs_to_jiffies(m);
}
first case with a non-const
main:
.LFB13:
.cfi_startproc
subq $8, %rsp #,
.cfi_def_cfa_offset 16
movq (%rsi), %rdi # *argv_1(D),
xorl %eax, %eax #
call atoi #
movl $1717986919, %edx #, tmp68
movl %eax, %ecx #, m
movl $.LC0, %edi #,
imull %edx # tmp68
sarl $31, %ecx #, tmp70
xorl %eax, %eax #
sarl $2, %edx #, tmp66
subl %ecx, %edx # tmp70, tmp66
movslq %edx, %rsi # tmp66, tmp71
call printf #
second with a constant:
main:
.LFB13:
.cfi_startproc
subq $8, %rsp #,
.cfi_def_cfa_offset 16
xorl %esi, %esi #
movl $.LC0, %edi #,
xorl %eax, %eax #
call printf #
giving it another run from scratch somewhere I simply screwed up or
overlooked some detail.
thx!
hofrat
next prev parent reply other threads:[~2015-04-06 6:40 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-05 7:23 [PATCH 0/3] time: use __builtin_constant_p() in msecs_to_jiffies Nicholas Mc Guire
2015-04-05 7:23 ` [PATCH 1/3] time: move timeconst.h into include/generated Nicholas Mc Guire
2015-04-05 7:23 ` [PATCH 2/3] time: allow gcc to fold constants when using msecs_to_jiffies Nicholas Mc Guire
2015-04-06 0:03 ` Joe Perches
2015-04-06 1:00 ` Nicholas Mc Guire
2015-04-06 2:15 ` Joe Perches
2015-04-06 4:26 ` Nicholas Mc Guire
2015-04-06 4:33 ` Joe Perches
2015-04-06 6:40 ` Nicholas Mc Guire [this message]
2015-04-06 7:12 ` Joe Perches
2015-04-06 7:21 ` Nicholas Mc Guire
2015-04-12 8:36 ` Nicholas Mc Guire
2015-04-05 7:23 ` [PATCH 3/3] time: update msecs_to_jiffies doc and move to kernel-doc format Nicholas Mc Guire
2015-04-05 9:33 ` [PATCH 0/3] time: use __builtin_constant_p() in msecs_to_jiffies Joe Perches
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=20150406064005.GA32350@opentech.at \
--to=der.herr@hofr.at \
--cc=ahh@google.com \
--cc=hofrat@osadl.org \
--cc=hpa@zytor.com \
--cc=joe@perches.com \
--cc=john.stultz@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mmarek@suse.cz \
--cc=pjt@google.com \
--cc=sam@ravnborg.org \
--cc=tglx@linutronix.de \
--cc=yamada.m@jp.panasonic.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox