This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] rs6000: Fix PR67344
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Alan Modra <amodra at gmail dot com>
- Cc: gcc-patches at gcc dot gnu dot org, dje dot gcc at gmail dot com
- Date: Tue, 25 Aug 2015 19:09:48 -0500
- Subject: Re: [PATCH] rs6000: Fix PR67344
- Authentication-results: sourceware.org; auth=none
- References: <cbc6c1ae8a15d8a41838eb0263f4d6bf98978668 dot 1440521500 dot git dot segher at kernel dot crashing dot org> <20150825231049 dot GC6569 at bubble dot grove dot modra dot org>
On Wed, Aug 26, 2015 at 08:40:49AM +0930, Alan Modra wrote:
> On Tue, Aug 25, 2015 at 10:08:54AM -0700, Segher Boessenkool wrote:
> > -(define_insn_and_split "*and<mode>3_imm_dot_shifted"
> > - [(set (match_operand:CC 3 "cc_reg_operand" "=x,?y")
> > +(define_insn "*and<mode>3_imm_dot_shifted"
> > + [(set (match_operand:CC 3 "cc_reg_operand" "=x")
>
> Is this really the best solution? The operand predicate allows any
> cr, but the constraint only cr0. In the past we've seen this sort of
> thing result in "insn does not satisfy its constraints" errors,
I don't think that can happen here. It is the same situation as
gpc_reg_operand with a "b" constraint.
> and if
> the operand is successfully reloaded you'll get slow mcrf insns.
What is slow about those? It's not mfcr :-)
Also note that prior compilers (at least as far back as 4.7) generated
it here, too (at -O2 etc. even).
> At
> -O1 the testcase generates:
>
> andi. 8,3,0x16
> mcrf 4,0
>
> I started throwing together a patch yesterday, before you claimed the
> bug. With this patch, I see what looks to be better code despite it
> being larger:
>
> li 9,22
> and 9,3,9
> cmpdi 4,9,0
The alternative that I was trying to avoid is
andi. 8,3,0x16
cmpdi 4,8,0
which doesn't have such a long dependency chain.
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 87abd6e..a9eea80 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -3060,17 +3060,18 @@
> return "#";
> }
> "&& reload_completed && cc_reg_not_cr0_operand (operands[3], CCmode)"
> - [(set (match_dup 0)
> - (and:GPR (lshiftrt:GPR (match_dup 1)
> - (match_dup 4))
> - (match_dup 2)))
> + [(set (match_dup 0) (match_dup 2))
This won't work for 0xa000 etc.
> + (set (match_dup 0) (and:GPR (match_dup 1) (match_dup 0)))
> (set (match_dup 3)
> (compare:CC (match_dup 0)
> (const_int 0)))]
> - ""
> + "
> +{
> + operands[2] = GEN_INT (UINTVAL (operands[2]) << INTVAL (operands[4]));
> +}"
> [(set_attr "type" "logical")
> (set_attr "dot" "yes")
> - (set_attr "length" "4,8")])
> + (set_attr "length" "4,12")])
Segher