[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 acsawdey at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #11 from acsawdey at gcc dot gnu.org --- This is fixed in trunk and gcc-8-branch. Hopefully I got this into 8 in time for it to get into 8.3.
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #10 from acsawdey at gcc dot gnu.org --- Author: acsawdey Date: Sat Feb 9 17:11:06 2019 New Revision: 268725 URL: https://gcc.gnu.org/viewcvs?rev=268725&root=gcc&view=rev Log: 2019-02-09 Aaron Sawdey Backported from mainline 2019-02-05 Aaron Sawdey PR target/89112 * config/rs6000/rs6000.md (tf_): Generate a local label for the long branch case. 2019-02-05 Aaron Sawdey PR target/89112 * config/rs6000/rs6000-string.c (do_ifelse, expand_cmp_vec_sequence, expand_compare_loop, expand_block_compare_gpr, expand_strncmp_align_check, expand_strncmp_gpr_sequence): Insert REG_BR_PROB notes in inline expansion of memcmp/strncmp. Add #include "profile-count.h" and "predict.h" for types and functions needed to work with REG_BR_PROB notes. 2019-02-09 Aaron Sawdey * config/rs6000/rs6000-string.c (expand_compare_loop, expand_block_compare): Insert REG_BR_PROB notes in inline expansion of memcmp/strncmp. Modified: branches/gcc-8-branch/gcc/ChangeLog branches/gcc-8-branch/gcc/config/rs6000/rs6000-string.c branches/gcc-8-branch/gcc/config/rs6000/rs6000.md
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #9 from acsawdey at gcc dot gnu.org --- The fixes for this are in trunk now. I will backport to gcc-8-branch in a week and then this can be closed.
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #8 from acsawdey at gcc dot gnu.org --- Author: acsawdey Date: Tue Feb 5 16:32:06 2019 New Revision: 268547 URL: https://gcc.gnu.org/viewcvs?rev=268547&root=gcc&view=rev Log: 2019-02-05 Aaron Sawdey PR target/89112 * config/rs6000/rs6000-string.c (do_ifelse, expand_cmp_vec_sequence, expand_compare_loop, expand_block_compare_gpr, expand_strncmp_align_check, expand_strncmp_gpr_sequence): Insert REG_BR_PROB notes in inline expansion of memcmp/strncmp. Add #include "profile-count.h" and "predict.h" for types and functions needed to work with REG_BR_PROB notes. Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/rs6000-string.c
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #7 from acsawdey at gcc dot gnu.org --- Author: acsawdey Date: Tue Feb 5 16:30:45 2019 New Revision: 268546 URL: https://gcc.gnu.org/viewcvs?rev=268546&root=gcc&view=rev Log: 2019-02-05 Aaron Sawdey PR target/89112 * config/rs6000/rs6000.md (tf_): Generate a local label for the long branch case. Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/rs6000.md
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #6 from Segher Boessenkool --- The patch in #c5 is pre-approved everywhere. Thanks! #c4... Do you *want* to keep it together? Is it in fact cold? If it is not, maybe you can improve the execution estimate for it?
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112
--- Comment #5 from acsawdey at gcc dot gnu.org ---
This patch fixes the issue on trunk:
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 268403)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -12639,8 +12639,8 @@
else
{
static char seq[96];
- char *bcs = output_cbranch (operands[3], "$+8", 1, insn);
- sprintf(seq, " $+12\;%s;b %%l0", bcs);
+ char *bcs = output_cbranch (operands[3], ".L%=", 1, insn);
+ sprintf(seq, " .L%%=\;%s\;b %%l0\;.L%%=:", bcs);
return seq;
}
}
I'm testing now, I will get this posted. Once approved for backport I'll apply
the same thing to gcc-8-branch for inclusion in the next 8 release.
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #4 from acsawdey at gcc dot gnu.org --- Well I can't blame this one on the linker or optimization. The splitting for the case where the branch destination is too far is wrong in tf_: static char seq[96]; char *bcs = output_cbranch (operands[3], "$+8", 1, insn); sprintf(seq, " $+12\;%s;b %%l0", bcs); return seq; This is wrong in both gcc 8 and 9. I'll get this fixed right away. The longer term question is how do I convince gcc to keep the code for a memcmp expansion together? I think this is happening because it thinks some of the code is cold and is throwing it at the end of the function.
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #3 from acsawdey at gcc dot gnu.org --- It appears that gcc decided to split the bdnzt generated by the memcmp expansion because the destination was out of range, and produced this: bdz $+12 beq 0,$+8 b $+8;b .L939 bne 0,.L937 ; --> to setb code So after the second iteration the bdz should branch to the bne which branches to a setb if there was a difference or falls through and does an overlapping compare to get the last 4 bytes of the 36 being compared. But the disassembly when I look at things in gdb has an extra branch in there which messes things up: 0x10008b90 : bdz 0x10008b9c 0x10008b94 : beq 0x10008b9c 0x10008b98 : b 0x10008ba0 0x10008b9c : b 0x1b8c 0x10008ba0 : bne 0x1bac So now the bdz branches to a branch to b8c which is back to the top of the loop to compare another 16 bytes which is of course wrong. It's possible this all happened because I didn't generate labels in the splitter, so multiple conditional branches had to be split because they were out of range.
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #2 from acsawdey at gcc dot gnu.org --- I'm seeing this on both gcc-8-branch and trunk, but only with -mcpu=power9. I'll figure out what happened here and get it fixed in trunk then back ported to 8.
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 acsawdey at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2019-01-30 Assignee|unassigned at gcc dot gnu.org |acsawdey at gcc dot gnu.org Ever confirmed|0 |1
[Bug target/89112] Incorrect code generated by rs6000 memcmp expansion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89112 --- Comment #1 from Samuel Holland --- Created attachment 45563 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45563&action=edit Output of gcc -v
