PowerPC: Define _STRING_ARCH_unaligned
diff mbox

Message ID 532B32B2.3020306@linux.vnet.ibm.com
State New
Headers show

Commit Message

Adhemerval Zanella March 20, 2014, 6:25 p.m. UTC
Here is another warning fixing, powerpc specific. Following where it is
defined for other archs, I had added a new header for PowerPC. I am not
very found of adding another arch-specific header, however it seems the
way to avoid such define issues.

--

2014-03-20  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>

	* sysdeps/powerpc/bits/string.h: New file: define _STRING_ARCH_unaligned.

---

Comments

Roland McGrath March 21, 2014, 8:28 p.m. UTC | #1
> Here is another warning fixing, powerpc specific. Following where it is
> defined for other archs, I had added a new header for PowerPC. I am not
> very found of adding another arch-specific header, however it seems the
> way to avoid such define issues.

We should not need a new header just for this.  In this case, it seems
appropriate for the generic bits/string.h to provide the common default.
It looks like the most common value is 1, but I'm not entirely sure.  We
should take a poll of all the architectures.  If there are only a few that
want 0, then I'd say those few can add their own bits/string.h that defines
it that way even if they have nothing else to define.  (For example today
sysdeps/m68k/m680x0/m68020/bits/string.h is nothing but setting it to 1.
So if 1 is the common default, that file would be removed and arm, ia64, et
al--whatever minority wants 0--would add files to set it to 0.)

If in fact there is not a big disparity between the number that want it set
to 0 but have no other bits/string.h content to provide and the number that
want it set to 1 but have no other bits/string.h content to provide, then
we should reconsider.  In that sort of case it may well be most reasonable
(i.e. best overall for avoiding typo-prone code balanced with avoiding
duplicative code) to have string.h do #ifndef #define after including
bits/string.h.

Note that your patch is not just fixing the warning, it is changing the
behavior because the missing macro was interpreted as 0 before.  So I think
we should treat this as two separate issues and possibly two separate
changes: -Wundef cleanup; and changing the powerpc setting.


Thanks,
Roland
Joseph Myers March 21, 2014, 10:37 p.m. UTC | #2
On Fri, 21 Mar 2014, Roland McGrath wrote:

> We should not need a new header just for this.  In this case, it seems
> appropriate for the generic bits/string.h to provide the common default.
> It looks like the most common value is 1, but I'm not entirely sure.  We

I think the common case, in terms of number of architectures, is probably 
0.  As I understand it, any architecture for which the GCC port defines 
STRICT_ALIGNMENT to 1, or SLOW_UNALIGNED_ACCESS to evaluate to 1 for the 
modes for which glibc can use unaligned accesses, should define 
_STRING_ARCH_unaligned to 0.  (In cases where it depends on compiler 
options, a more careful examination is needed to work out what variants 
are actually relevant for glibc - it's quite possible that for some such 
architectures, all processors supported by glibc support unaligned 
operations for the relevant types.)
Roland McGrath March 21, 2014, 10:51 p.m. UTC | #3
> I think the common case, in terms of number of architectures, is probably 
> 0.

That is functionally consistent with the current code, where the generic
file omits the macro.  But let us measure rather than guess.  Let someone
(the arch maintainer or whoever gets to it) say for each arch what the
proper value is, and let someone collate all those results.

Patch
diff mbox

diff --git a/sysdeps/powerpc/bits/string.h b/sysdeps/powerpc/bits/string.h
new file mode 100644
index 0000000..eff3155
--- /dev/null
+++ b/sysdeps/powerpc/bits/string.h
@@ -0,0 +1,24 @@ 
+/* Optimized inlined string functions.  PowerPC version.
+   Copyright (C) 2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _STRING_H
+# error "Never use <bits/string.h> directly; include <string.h> instead."
+#endif
+
+#define _STRING_ARCH_unaligned	1