public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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: Sun, 12 Apr 2015 10:36:34 +0200	[thread overview]
Message-ID: <20150412083634.GA19486@opentech.at> (raw)
In-Reply-To: <1428286502.2775.92.camel@perches.com>

> Your inline version has not worked with any of
> x86-64 gcc 4.4, 4.6, 4.7, or 4.9
> 
> I suggest you add some lines to
> lib/test_module.c/test_module_init like:
> 
> 	unsigned int m;
> 
> 	for (m = 10; m < 200; m += 10)
> 		pr_info("msecs_to_jiffies(%u) is %lu\n",
> 			m, msecs_to_jiffies(m));
> 
> 	pr_info("msecs_to_jiffies(%u) is %lu\n",
> 		10, msecs_to_jiffies(10));
> 	pr_info("msecs_to_jiffies(%u) is %lu\n",
> 		100, msecs_to_jiffies(100));
> 	pr_info("msecs_to_jiffies(%u) is %lu\n",
> 		1000, msecs_to_jiffies(1000));
> 
> Then it's pretty easy to look at the assembly/.lst file
> 
> Your inline function doesn't allow gcc to precompute
> the msecs_to_jiffies value.  The macro one does for all
> those gcc versions.
> 
> Try it and look at the generated .lst files with and
> without the patch I sent.
> 
I have checked it with the testcase you proposed  - and I quite sure
it is working, find the .s file snippets from the test_module.c modified 
as you suggested below. HZ is set to 300 so that it is easy to differenciate
the parameter to msecs_to_jiffies and the printed const.

there is no call and no calculation for the constant cases - just load as 
it should be.

with the patch applied:
test_module_init:
        pushq   %rbp    #
        movq    %rsp, %rbp      #,
        pushq   %rbx    #
        movl    $10, %ebx       #, m
        pushq   %rcx    #
.L4:
        movl    %ebx, %edi      # m,
        call    __msecs_to_jiffies      #
        movl    %ebx, %esi      # m,
        movq    %rax, %rdx      # D.14515,
        movq    $.LC1, %rdi     #,
        xorl    %eax, %eax      #
        addl    $10, %ebx       #, m
        call    printk  #
        cmpl    $200, %ebx      #, m
        jne     .L4     #,                <---end of for loop
        movl    $3, %edx        #,        <---msecs_to_jiffies(10)
        movl    $10, %esi       #,
        movq    $.LC1, %rdi     #,
        xorl    %eax, %eax      #
        call    printk  #
        movl    $30, %edx       #,        <---msecs_to_jiffies(100)
        movl    $100, %esi      #,
        movq    $.LC1, %rdi     #,
        xorl    %eax, %eax      #
        call    printk  #
        movl    $300, %edx      #,        <---msecs_to_jiffies(1000)
        movl    $1000, %esi     #,
        movq    $.LC1, %rdi     #,
        xorl    %eax, %eax      #
        call    printk  #

without the patch applied:

        pushq   %rbp    #
        movq    %rsp, %rbp      #,
        pushq   %rbx    #
        movl    $10, %ebx       #, m
        pushq   %rcx    #
.L2:
        movl    %ebx, %edi      # m,
        call    msecs_to_jiffies        #
        movl    %ebx, %esi      # m,
        movq    %rax, %rdx      # D.14503,
        movq    $.LC0, %rdi     #,
        xorl    %eax, %eax      #
        addl    $10, %ebx       #, m
        call    printk  #
        cmpl    $200, %ebx      #, m
        jne     .L2     #,
        movl    $10, %edi       #,
        call    msecs_to_jiffies        #
        movl    $10, %esi       #,
        movq    %rax, %rdx      # D.14504,
        movq    $.LC0, %rdi     #,
        xorl    %eax, %eax      #
        call    printk  #
        movl    $100, %edi      #,
        call    msecs_to_jiffies        #
        movl    $100, %esi      #,
        movq    %rax, %rdx      # D.14505,
        movq    $.LC0, %rdi     #,
        xorl    %eax, %eax      #
        call    printk  #
        movl    $1000, %edi     #,

could you check the .s file for you test module ?
I'm a bit lost on why you are not seeing this - also
checked with cross-build (multi_v7_defconfig) and it
looks like thats working as well.

thx!
hofrat

  parent reply	other threads:[~2015-04-12  8:36 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
2015-04-06  7:12               ` Joe Perches
2015-04-06  7:21                 ` Nicholas Mc Guire
2015-04-12  8:36         ` Nicholas Mc Guire [this message]
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=20150412083634.GA19486@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