unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: [Emacs-diffs] trunk r114450: * dispnew.c (clear_glyph_row, copy_row_except_pointers):
       [not found] <E1VOMrf-0001CU-O0@vcs.savannah.gnu.org>
@ 2013-09-24 11:43 ` Daniel Colascione
  2013-09-24 15:26   ` Paul Eggert
  2013-09-24 12:19 ` Dmitry Antipov
  1 sibling, 1 reply; 4+ messages in thread
From: Daniel Colascione @ 2013-09-24 11:43 UTC (permalink / raw)
  To: Paul Eggert, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On 9/24/13 12:16 AM, Paul Eggert wrote:
> ------------------------------------------------------------
> revno: 114450
> revision-id: eggert@cs.ucla.edu-20130924071638-9k6809ka6hbocr2n
> parent: dmantipov@yandex.ru-20130924064320-9o1cx41btsnt1voo
> committer: Paul Eggert <eggert@cs.ucla.edu>
> branch nick: trunk
> timestamp: Tue 2013-09-24 00:16:38 -0700
> message:
>   * dispnew.c (clear_glyph_row, copy_row_except_pointers):
>   
>   Prefer signed to unsigned integers where either will do.

Why? Signed integers have undefined overflow behavior and sometimes
result in less efficient code. If anything, we should prefer unsigned
integer types.

int
with_signed (int arg)
{
    return arg/64;
}


unsigned
with_unsigned (unsigned arg)
{
    return arg/64;
}

$ gcc-mp-4.7 -O2 -march=core2 foo.c
$ gdb a.out
Reading symbols for shared libraries ... done
(gdb) disassemble with_signed
Dump of assembler code for function with_signed:
0x0000000100000f40 <with_signed+0>:	lea    0x3f(%rdi),%eax
0x0000000100000f43 <with_signed+3>:	test   %edi,%edi
0x0000000100000f45 <with_signed+5>:	cmovns %edi,%eax
0x0000000100000f48 <with_signed+8>:	sar    $0x6,%eax
0x0000000100000f4b <with_signed+11>:	retq
0x0000000100000f4c <with_signed+12>:	nopl   0x0(%rax)
End of assembler dump.
(gdb) disassemble with_unsigned
Dump of assembler code for function with_unsigned:
0x0000000100000f50 <with_unsigned+0>:	mov    %edi,%eax
0x0000000100000f52 <with_unsigned+2>:	shr    $0x6,%eax
0x0000000100000f55 <with_unsigned+5>:	retq
End of assembler dump.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Emacs-diffs] trunk r114450: * dispnew.c (clear_glyph_row, copy_row_except_pointers):
       [not found] <E1VOMrf-0001CU-O0@vcs.savannah.gnu.org>
  2013-09-24 11:43 ` [Emacs-diffs] trunk r114450: * dispnew.c (clear_glyph_row, copy_row_except_pointers): Daniel Colascione
@ 2013-09-24 12:19 ` Dmitry Antipov
  2013-09-24 15:45   ` Paul Eggert
  1 sibling, 1 reply; 4+ messages in thread
From: Dmitry Antipov @ 2013-09-24 12:19 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

On 9/24/13 12:16 AM, Paul Eggert wrote:

> ------------------------------------------------------------
> revno: 114450
> revision-id: eggert@cs.ucla.edu-20130924071638-9k6809ka6hbocr2n
> parent: dmantipov@yandex.ru-20130924064320-9o1cx41btsnt1voo
> committer: Paul Eggert <eggert@cs.ucla.edu>
> branch nick: trunk
> timestamp: Tue 2013-09-24 00:16:38 -0700
> message:
>   * dispnew.c (clear_glyph_row, copy_row_except_pointers):
>
>   Prefer signed to unsigned integers where either will do.

Since offsetof returns size_t, portable code should not 1) assume
that sizeof (int) == sizeof (size_t) and 2) assume that
offsetof (type, member) is always less than INT_MAX, isn't it?

 >   Omit easserts with unnecessary and unportable assumptions about
 >   alignment.

It would be nice to illustrate this thesis with an example of the
architecture/ABI where these assertions doesn't work as expected.

Dmitry




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Emacs-diffs] trunk r114450: * dispnew.c (clear_glyph_row, copy_row_except_pointers):
  2013-09-24 11:43 ` [Emacs-diffs] trunk r114450: * dispnew.c (clear_glyph_row, copy_row_except_pointers): Daniel Colascione
@ 2013-09-24 15:26   ` Paul Eggert
  0 siblings, 0 replies; 4+ messages in thread
From: Paul Eggert @ 2013-09-24 15:26 UTC (permalink / raw)
  To: Daniel Colascione, emacs-devel

Daniel Colascione wrote:
> Signed integers have undefined overflow behavior and sometimes
> result in less efficient code. If anything, we should prefer unsigned
> integer types.

The general Emacs style is to prefer signed integers to unsigned.
That's why the Emacs source code prefers ptrdiff_t to size_t, for
example.  This is not a style that I originally favored, but I've
learned to like its advantages.  Most importantly, it leads to
more-understandable code, since signed arithmetic is closer to how
people normally understand numbers: it's counterintutive that a
large positive number X can satisfy the predicate (X < -1), which
can happen with unsigned arithmetic.  And as a side effect, signed
arithmetic often leads to more-efficient code, since the compiler
can take advantage of the fact that behavior is undefined on
overflow.

There are a few exceptions to the general style; one is bitmasks
(where the integers are not really numbers, but are bit strings).
Another is char, where unsigned char is often used, as it's
simpler (and typically, this doesn't have the problem with -1
anyway).  Still another are hash values and a few other places
where the performance advantage of unsigned arithmetic is
compelling.  But generally, we prefer signed integers.




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Emacs-diffs] trunk r114450: * dispnew.c (clear_glyph_row, copy_row_except_pointers):
  2013-09-24 12:19 ` Dmitry Antipov
@ 2013-09-24 15:45   ` Paul Eggert
  0 siblings, 0 replies; 4+ messages in thread
From: Paul Eggert @ 2013-09-24 15:45 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: emacs-devel

Dmitry Antipov wrote:
> portable code should not 1) assume
> that sizeof (int) == sizeof (size_t) and 2) assume that
> offsetof (type, member) is always less than INT_MAX, isn't it?

Well, the usual style elsewhere in Emacs is to use enums
for offsetof-related constants, so I did that in trunk bzr 114453.
This has the minor technical advantage of requiring a compiler
diagnostic (as opposed to relying on undefined behavior) if
the offsetof value exceeded INT_MAX.  I suppose that might happen
due to compiler misconfiguration; it should never happen in any
remotely-plausible computer, but it doesn't hurt to check.

As for alignment, I once dealt with a real Unix machine that
had 3-byte words that were always aligned on 4-byte boundaries.
Adding 1 to a char * value added 1 to its internal representation
two times out of three; the other time, it added 2, to skip over
the byte that wasn't there.  Admittedly this was a while ago,
but it did teach me to mistrust assumptions about alignment.
In this case the unportable assumption cluttered up the code
and led to confusion about what the code really was assuming,
and it wasn't worth adding the complexity to the assertion to
make it state clearly what the assumption was, so I thought
it simpler to remove it.



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-09-24 15:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <E1VOMrf-0001CU-O0@vcs.savannah.gnu.org>
2013-09-24 11:43 ` [Emacs-diffs] trunk r114450: * dispnew.c (clear_glyph_row, copy_row_except_pointers): Daniel Colascione
2013-09-24 15:26   ` Paul Eggert
2013-09-24 12:19 ` Dmitry Antipov
2013-09-24 15:45   ` Paul Eggert

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).