Patchwork [v3] libstdc++/44413 (for ext/vstring)

login
register
mail settings
Submitter Paolo Carlini
Date June 9, 2010, 9:22 a.m.
Message ID <4C0F5D67.9010405@oracle.com>
Download mbox | patch
Permalink /patch/55059/
State New
Headers show

Comments

Paolo Carlini - June 9, 2010, 9:22 a.m.
Hi,

let's do this for ext/vstring first, and let's see how people react. I'm
still reluctant to do it for basic_string, given that the compare
functions are exported and legacy apps are at risk of behaving
differently all of a sudden. Committed to mainline.

Thanks,
Paolo.

///////////////
2010-06-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR libstdc++/44413
	* include/ext/vstring_util.h (__vstring_utility<>::_S_compare):
	Simplify, just return -1, 0, 1.
Doug Semler - June 9, 2010, 1:52 p.m.
On Wed, Jun 9, 2010 at 5:22 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> let's do this for ext/vstring first, and let's see how people react. I'm
> still reluctant to do it for basic_string, given that the compare
> functions are exported and legacy apps are at risk of behaving
> differently all of a sudden. Committed to mainline.
>

I don't see it.  You're really hurting the 32 bit side with this.  The
original is reduced to a subtraction, while the new code adds:

        xorl    %eax, %eax
        cmpl    $0, %edx
        je      .L3
        setg    %al
        movzbl  %al, %eax
        leal    -1(%eax,%eax), %eax
.L3
        ret

In addition, it seems that the attempt only reduces the code by one move?

Patch

Index: include/ext/vstring_util.h
===================================================================
--- include/ext/vstring_util.h	(revision 160455)
+++ include/ext/vstring_util.h	(working copy)
@@ -1,6 +1,7 @@ 
 // Versatile string utility -*- C++ -*-
 
-// Copyright (C) 2005, 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
+// Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010
+// Free Software Foundation, Inc.
 //
 // This file is part of the GNU ISO C++ Library.  This library is free
 // software; you can redistribute it and/or modify it under the
@@ -50,10 +51,10 @@ 
     {
       typedef typename _Alloc::template rebind<_CharT>::other _CharT_alloc_type;
 
-      typedef _Traits					    traits_type;      
+      typedef _Traits					    traits_type;
       typedef typename _Traits::char_type		    value_type;
       typedef typename _CharT_alloc_type::size_type	    size_type;
-      typedef typename _CharT_alloc_type::difference_type   difference_type;      
+      typedef typename _CharT_alloc_type::difference_type   difference_type;
       typedef typename _CharT_alloc_type::pointer	    pointer;
       typedef typename _CharT_alloc_type::const_pointer	    const_pointer;
 
@@ -164,14 +165,8 @@ 
       static int
       _S_compare(size_type __n1, size_type __n2)
       {
-	const difference_type __d = difference_type(__n1 - __n2);
-
-	if (__d > __numeric_traits_integer<int>::__max)
-	  return __numeric_traits_integer<int>::__max;
-	else if (__d < __numeric_traits_integer<int>::__min)
-	  return __numeric_traits_integer<int>::__min;
-	else
-	  return int(__d);
+	const difference_type __d = __n1 - __n2;
+	return __d == 0 ? 0 : (__d > 0 ? 1 : -1);
       }
     };