This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.
Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
This patch undoes Franz's temporary fix and introduces a permanent fix
that will work for BP and non-BP. It passes string/tester.
I changed the way strchr returns NULL:
Before: We zeroed the return-value register, then in two places
conditionally branched to LR.
After: In two places, we conditionally branch to a label that
zeroes the return-value register then unconditionally branches
to LR.
This adds one insn overall to the size.
It adds one extra unconditional branch to the return-NULL case.
It saves a register move for the return-non-NULL case.
IMO, it's a basically wash.
The primary virtue of this approach is that it avoids all #ifs
on __BOUNDED_POINTERS__ enhancing maintainability.
What follows immediately for review is a patch relative to the
previous version (before Franz's temporary fix), so as to not obscure
the meaningful parts with the reversal of the temporary fix. (The
complete patch is at the end.)
OK?
Greg
Index: sysdeps/powerpc/strchr.S
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/powerpc/strchr.S,v
retrieving revision 1.4
diff -u -p -B -w -r1.4 strchr.S
--- strchr.S 2000/07/04 00:24:42 1.4
+++ strchr.S 2000/07/22 08:34:22
@@ -88,11 +89,9 @@ L(loopentry):
before or after the zero byte). In fact, we may be looking for a
zero byte, in which case we return a match. We guess that this hasn't
happened, though. */
-L(missed):
and. rTMP1, rTMP1, rTMP2
- li rSTR, 0
- STORE_RETURN_VALUE (rSTR)
- beqlr
+ beq L(return_NULL)
+
/* It did happen. Decide which one was first...
I'm not sure if this is actually faster than a sequence of
rotates, compares, and branches (we use it anyway because it's shorter). */
@@ -105,7 +104,8 @@ L(missed):
nor rWORD, rMASK, rFEFE
nor rTMP2, rIGN, rTMP1
cmplw rWORD, rTMP2
- bgtlr
+ bgt L(return_NULL)
+
cntlzw rCLZB, rTMP2
srwi rCLZB, rCLZB, 3
add rSTR, rSTR, rCLZB
@@ -113,6 +113,7 @@ L(missed):
STORE_RETURN_VALUE (rSTR)
blr
+/* We found a CHR byte in WORD, and there is no 00 byte present in WORD. */
L(foundit):
and rTMP1, r7F7F, rTMP3
or rIGN, r7F7F, rTMP3
@@ -125,6 +126,12 @@ L(foundit):
CHECK_BOUNDS_HIGH_RTN (rSTR, rTMP2, twlge)
STORE_RETURN_VALUE (rSTR)
blr
+
+L(return_NULL):
+ li rSTR, 0
+ STORE_RETURN_VALUE (rSTR)
+ blr
+
END (BP_SYM (strchr))
weak_alias (BP_SYM (strchr), BP_SYM (index))
------------------------------------------------------------------------------
This is the complete patch that includes backing out the temporary
fix.
Index: sysdeps/powerpc/strchr.S
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/powerpc/strchr.S,v
retrieving revision 1.5
diff -u -p -B -w -r1.5 strchr.S
--- strchr.S 2000/07/22 02:25:05 1.5
+++ strchr.S 2000/07/22 08:34:50
@@ -34,7 +34,7 @@ ENTRY (BP_SYM (strchr))
# define rCHR r5 /* byte we're looking for, spread over the whole word */
# define rWORD r8 /* the current word */
#else
-# define rSTR r8 /* current word pointer */
+# define rSTR r3 /* current word pointer */
# define rCHR r4 /* byte we're looking for, spread over the whole word */
# define rWORD r5 /* the current word */
#endif
@@ -52,10 +52,10 @@ ENTRY (BP_SYM (strchr))
rlwimi rCHR, rCHR, 8, 16, 23
li rMASK, -1
rlwimi rCHR, rCHR, 16, 0, 15
- rlwinm rIGN, rRTN, 3, 27, 28
+ rlwinm rIGN, rSTR, 3, 27, 28
lis rFEFE, -0x101
lis r7F7F, 0x7f7f
- clrrwi rSTR, rRTN, 2
+ clrrwi rSTR, rSTR, 2
addi rFEFE, rFEFE, -0x101
addi r7F7F, r7F7F, 0x7f7f
/* Test the first (partial?) word. */
@@ -88,11 +89,9 @@ L(loopentry):
before or after the zero byte). In fact, we may be looking for a
zero byte, in which case we return a match. We guess that this hasn't
happened, though. */
-L(missed):
and. rTMP1, rTMP1, rTMP2
- li rRTN, 0
- STORE_RETURN_VALUE (rSTR)
- beqlr
+ beq L(return_NULL)
+
/* It did happen. Decide which one was first...
I'm not sure if this is actually faster than a sequence of
rotates, compares, and branches (we use it anyway because it's shorter). */
@@ -105,14 +104,16 @@ L(missed):
nor rWORD, rMASK, rFEFE
nor rTMP2, rIGN, rTMP1
cmplw rWORD, rTMP2
- bgtlr
+ bgt L(return_NULL)
+
cntlzw rCLZB, rTMP2
srwi rCLZB, rCLZB, 3
- add rRTN, rSTR, rCLZB
+ add rSTR, rSTR, rCLZB
CHECK_BOUNDS_HIGH_RTN (rSTR, rTMP2, twlge)
STORE_RETURN_VALUE (rSTR)
blr
+/* We found a CHR byte in WORD, and there is no 00 byte present in WORD. */
L(foundit):
and rTMP1, r7F7F, rTMP3
or rIGN, r7F7F, rTMP3
@@ -121,10 +122,16 @@ L(foundit):
cntlzw rCLZB, rTMP2
subi rSTR, rSTR, 4
srwi rCLZB, rCLZB, 3
- add rRTN, rSTR, rCLZB
+ add rSTR, rSTR, rCLZB
CHECK_BOUNDS_HIGH_RTN (rSTR, rTMP2, twlge)
STORE_RETURN_VALUE (rSTR)
blr
+
+L(return_NULL):
+ li rSTR, 0
+ STORE_RETURN_VALUE (rSTR)
+ blr
+
END (BP_SYM (strchr))
weak_alias (BP_SYM (strchr), BP_SYM (index))
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |