On Sat, Feb 02, 2019 at 05:17:31PM -0600, Aaron Sawdey wrote:
> I needed to introduce a local label in this pattern because output_cbranch
> put out a second instruction
> in the long branch case. This fixes the issue but there are a couple ways
> this could be improved:
>
> * output_cbranch() is passed the original insn and assumes from that that the
> branch is a long
> branch. However this is incorrect because we are just branching to a local
> label we know is only
> a few instructions away. If there is a way to fix this, an unnecessary branch
> could be eliminated.
Maybe output_cbranch can be made aware of bdz{,t,f} and bdnz{,t,f}?
> * While the long branch case of this pattern needs to work, the real problem
> is that part of
> the code emitted by the memcmp expansion is being treated as cold code and
> moved to the end of
> the function. Ideally all of this code should stay together. I suspect I need
> to make some kind
> of branch frequency notation for this to happen.
You can emit a REG_BR_PROB note on the branches that need one?
> Regstrap passes on ppc64le power7/8/9, ok for trunk and backport to 8?
I pre-approved it in the PR, the messages crossed I think :-)
But, hrm. Labels ".L<number>" are already used for something else, with
something else for <number>. Please use a unique name? Okay with that
change. Thanks!
Segher
> 2019-02-02 Aaron Sawdey <acsaw...@linux.ibm.com>
>
> * config/rs6000/rs6000.md (<bd>tf_<mode>): generate a local label
> for the long branch case.
>
> 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, "<bd_neg> $+12\;%s;b %%l0", bcs);
> + char *bcs = output_cbranch (operands[3], ".L%=", 1, insn);
> + sprintf(seq, "<bd_neg> .L%%=\;%s\;b %%l0\;.L%%=:", bcs);
> return seq;
> }
> }
