public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Bernardo Innocenti <bernie@develer.com>
To: Richard Henderson <rth@twiddle.net>, Andrea Arcangeli <andrea@suse.de>
Cc: Linus Torvalds <torvalds@osdl.org>,
	linux-kernel@vger.kernel.org,
	Peter Chubb <peter@chubb.wattle.id.au>,
	Andrew Morton <akpm@digeo.com>, Ian Molton <spyro@f2s.com>,
	gcc@gcc.gnu.org
Subject: Re: [PATCH] Fix do_div() for all architectures
Date: Thu, 10 Jul 2003 21:31:45 +0200	[thread overview]
Message-ID: <200307102131.45474.bernie@develer.com> (raw)
In-Reply-To: <20030710163918.GB18697@twiddle.net>

On Thursday 10 July 2003 18:39, Richard Henderson wrote:

 > On Thu, Jul 10, 2003 at 06:18:59PM +0200, Andrea Arcangeli wrote:
 > > On Thu, Jul 10, 2003 at 08:40:19AM -0700, Richard Henderson wrote:
 > > > On Tue, Jul 08, 2003 at 08:27:26PM +0200, Bernardo Innocenti wrote:
 > > > > +extern uint32_t __div64_32(uint64_t *dividend, uint32_t
 > > > > divisor) __attribute_pure__;
 > > >
 > > > ...
 > > >
 > > > > +		__rem = __div64_32(&(n), __base);	\
 > > >
 > > > The pure declaration is very incorrect.  You're writing to N.
 > >
 > > now pure sounds more reasonable, I wondered how could gcc keep track
 > > of the stuff pointed by the parameters (especially if this stuff
 > > points to other stuff etc.. ;).

 The compiler could easily tell what memory can be clobbered by a pointer
by applying type-based aliasing rules. For example, a function taking a
"char *" can't clobber memory objects declared as "long bar" or
"struct foo".

 Without type based alias analysis, the compiler is forced to flush
all registers containing copies of memory objects before function
call and reloading values from memory afterwards.


 > Bernardo mis-interpreted the documentation. [...]

 I'm afraid you're right. Here's a code snippet from gcc/calls.c that
shows what the compiler _really_ does for pure calls:

  /* If the result of a pure or const function call is ignored (or void),
     and none of its arguments are volatile, we can avoid expanding the
     call and just evaluate the arguments for side-effects.  */
  if ((flags & (ECF_CONST | ECF_PURE))
      && (ignore || target == const0_rtx
          || TYPE_MODE (TREE_TYPE (exp)) == VOIDmode))
    {
      bool volatilep = false;
      tree arg;

      for (arg = actparms; arg; arg = TREE_CHAIN (arg))
        if (TREE_THIS_VOLATILE (TREE_VALUE (arg)))
          {
            volatilep = true;
            break;
          }

      if (! volatilep)
        {
          for (arg = actparms; arg; arg = TREE_CHAIN (arg))
            expand_expr (TREE_VALUE (arg), const0_rtx,
                         VOIDmode, EXPAND_NORMAL);
          return const0_rtx;
        }
    }


Therefore this optimization is to be undone. Would it work if
we could use references instead of pointers? I think it
wouldn't. A new attribute would be needed for this case.

Just to open some interesting speculation, do you think we'd
get better code by just getting rid of __attribute__((pure))
or by changing __do_div64() to do something like this?

 typedef struct { uint64_t quot, uint32_t rem } __quotrem64;
 __quotrem64 __do_div64(uint64_t div, uint32_t base) __attribute__((const));

 #define do_div(n,base) ({                                        \
        uint32_t __base = (base);                                 \
        uint32_t __rem;                                           \
        if (likely(((n) >> 32) == 0)) {                           \
                __rem = (uint32_t)(n) % __base;                   \
                (n) = (uint32_t)(n) / __base;                     \
        } else {                                                  \
                __quotrem64 __qr = __div64_32((n), __base);       \
                (n) = __qr.quot;                                  \
                __rem = __qr.rem;                                 \
        }                                                         \
        __rem;                                                    \
 })

Boy, that's ugly! It's too bad C can't do it the Perl way:

    (n,rem) = __div64_32(n, base);

-- 
  // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/

Please don't send Word attachments - http://www.gnu.org/philosophy/no-word-attachments.html



  reply	other threads:[~2003-07-10 19:17 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-07-05 23:33 [PATCH] Fix do_div() for all architectures Bernardo Innocenti
2003-07-06  7:47 ` Russell King
2003-07-06 15:40   ` Ian Molton
2003-07-07  4:26 ` Bernardo Innocenti
2003-07-08 18:27   ` Bernardo Innocenti
2003-07-08 18:31     ` Andrew Morton
2003-07-08 20:56       ` Bernardo Innocenti
2003-07-10 15:40     ` Richard Henderson
2003-07-10 16:18       ` Andrea Arcangeli
2003-07-10 16:39         ` Richard Henderson
2003-07-10 19:31           ` Bernardo Innocenti [this message]
2003-07-10 19:53             ` Dale Johannesen
2003-07-10 20:19             ` Andrea Arcangeli
2003-07-10 22:58               ` Bernardo Innocenti
2003-07-10 22:04             ` Richard Henderson
2003-07-10 23:13       ` Bernardo Innocenti
  -- strict thread matches above, loose matches on Subject: below --
2003-07-02 16:16 [PATCH] Kill div64.h dupes, parenthesize do_div() macro params Linus Torvalds
2003-07-03 10:43 ` [PATCH] Fix do_div() for all architectures Bernardo Innocenti

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=200307102131.45474.bernie@develer.com \
    --to=bernie@develer.com \
    --cc=akpm@digeo.com \
    --cc=andrea@suse.de \
    --cc=gcc@gcc.gnu.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter@chubb.wattle.id.au \
    --cc=rth@twiddle.net \
    --cc=spyro@f2s.com \
    --cc=torvalds@osdl.org \
    /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