Unfortunately this patch is eliciting a number of new testsuite
failures, all like
error: unrecognizable insn:
(insn 44 43 45 5 (parallel [
(set (reg:SI 199)
(unspec:SI [
(reg:SF 202)
] UNSPEC_SI_FROM_SF))
(clobber (scratch:V4SF))
])
"/nasfarm/edelsohn/src/src/gcc/testsuite/gcc.dg/vect/vect-alias-check-11.c":70:1
-1
(nil))
during RTL pass: vregs
for
gcc.dg/vect/vect-alias-check-11.c
gcc.dg/vect/vect-alias-check-12.c
gcc.dg/vect/pr57741-2.c
gcc.dg/vect/pr57741-3.c
gcc.dg/vect/pr89440.c
gcc.target/powerpc/sse-movss-1.c
Thanks, David
On Mon, Jul 13, 2020 at 2:30 AM luoxhu <luo...@linux.ibm.com> wrote:
>
> Hi,
>
> On 2020/7/11 08:54, Segher Boessenkool wrote:
> > Hi!
> >
> > On Fri, Jul 10, 2020 at 09:39:40AM +0800, luoxhu wrote:
> >> OK, seems the md file needs a format tool too...
> >
> > Heh. Just make sure it looks good (that is, does what it looks like),
> > looks like the rest, etc. It's hard to do anything nice with unspecs,
> > [ ] lists do not format well.
> >
> >>>> + "TARGET_NO_SF_SUBREG"
> >>>> + "#"
> >>>> + "&& vsx_reg_sfsubreg_ok (operands[0], SFmode)"
> >>>
> >>> Put this in the insn condition? And since this is just a predicate,
> >>> you can just use it instead of gpc_reg_operand.
> >>>
> >>> (The split condition becomes "&& 1" then, not "").
> >>
> >> OK, this seems a bit strange as movsi_from_sf, movsf_from_si and
> >> movdi_from_sf_zero_ext all use it as condition...
> >
> > Since in your case you *always* split, the split condition should be
> > "always". There are no alternatives that do not split here.
> >
> >> And why vsx_reg_sfsubreg_ok allows "SF SUBREGS" and TARGET_NO_SF_SUBREG
> >> "avoid (SUBREG:SF (REG:SI)", I guess they are not the same meaning? (The
> >> TARGET_NO_SF_SUBREG is also copied from other similar defines.) Thanks.
> >
> > Good question. I do not know.
> >
> > Well... Since this define_insn* requires p8 *anyway*, we do not need
> > any of these sf_subreg things? We always know for each one if it should
> > be true or false.
>
> Yes, removed the vsx_reg_sfsubreg_ok check.
>
> >
> >> + "TARGET_NO_SF_SUBREG"
> >
> > But here we should require p8 some other way, then.
>
> TARGET_NO_SF_SUBREG is defined to TARGET_DIRECT_MOVE_64BIT, and
> TARGET_DIRECT_MOVE_64BIT is TARGET_DIRECT_MOVE && TARGET_P8_VECTOR &&
> TARGET_POWERPC64
> which means TARGET_P8_VECTOR must be true for TARGET_NO_SF_SUBREG.
>
> >
> >> + (set_attr "isa" "p8v")])
> >
> > (This isn't enough, unfortunately).
> >
>
>
> Updated patch to removed the vsx_reg_sfsubreg_ok and ICE fix:
>
>
> For extracting high part element from DImode register like:
>
> {%1:SF=unspec[r122:DI>>0x20#0] 86;clobber scratch;}
>
> split it before reload with "and mask" to avoid generating shift right
> 32 bit then shift left 32 bit. This pattern also exists in PR42475 and
> PR67741, etc.
>
> srdi 3,3,32
> sldi 9,3,32
> mtvsrd 1,9
> xscvspdpn 1,1
>
> =>
>
> rldicr 3,3,0,31
> mtvsrd 1,3
> xscvspdpn 1,1
>
> Bootstrap and regression tested pass on Power8-LE.
>
> gcc/ChangeLog:
>
> 2020-07-13 Xionghu Luo <luo...@linux.ibm.com>
>
> PR rtl-optimization/89310
> * config/rs6000/rs6000.md (movsf_from_si2): New
> define_insn_and_split.
>
> gcc/testsuite/ChangeLog:
>
> 2020-07-13 Xionghu Luo <luo...@linux.ibm.com>
>
> PR rtl-optimization/89310
> * gcc.target/powerpc/pr89310.c: New test.
> ---
> gcc/config/rs6000/rs6000.md | 31 ++++++++++++++++++++++
> gcc/testsuite/gcc.target/powerpc/pr89310.c | 17 ++++++++++++
> 2 files changed, 48 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr89310.c
>
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 4fcd6a94022..480385ed4d2 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -7593,6 +7593,37 @@ (define_insn_and_split "movsf_from_si"
> "*, *, p9v, p8v, *, *,
> p8v, p8v, p8v, *")])
>
> +;; For extracting high part element from DImode register like:
> +;; {%1:SF=unspec[r122:DI>>0x20#0] 86;clobber scratch;}
> +;; split it before reload with "and mask" to avoid generating shift right
> +;; 32 bit then shift left 32 bit.
> +(define_insn_and_split "movsf_from_si2"
> + [(set (match_operand:SF 0 "gpc_reg_operand" "=wa")
> + (unspec:SF
> + [(subreg:SI
> + (ashiftrt:DI
> + (match_operand:DI 1 "input_operand" "r")
> + (const_int 32))
> + 0)]
> + UNSPEC_SF_FROM_SI))
> + (clobber (match_scratch:DI 2 "=r"))]
> + "TARGET_NO_SF_SUBREG"
> + "#"
> + "&& 1"
> + [(const_int 0)]
> +{
> + if (GET_CODE (operands[2]) == SCRATCH)
> + operands[2] = gen_reg_rtx (DImode);
> +
> + rtx mask = GEN_INT (HOST_WIDE_INT_M1U << 32);
> + emit_insn (gen_anddi3 (operands[2], operands[1], mask));
> + emit_insn (gen_p8_mtvsrd_sf (operands[0], operands[2]));
> + emit_insn (gen_vsx_xscvspdpn_directmove (operands[0], operands[0]));
> + DONE;
> +}
> + [(set_attr "length" "12")
> + (set_attr "type" "vecfloat")
> + (set_attr "isa" "p8v")])
>
> ;; Move 64-bit binary/decimal floating point
> (define_expand "mov<mode>"
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr89310.c
> b/gcc/testsuite/gcc.target/powerpc/pr89310.c
> new file mode 100644
> index 00000000000..15e78509246
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr89310.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +struct s {
> + int i;
> + float f;
> +};
> +
> +float
> +foo (struct s arg)
> +{
> + return arg.f;
> +}
> +
> +/* { dg-final { scan-assembler-not {\msrdi\M} } } */
> +/* { dg-final { scan-assembler-not {\msldi\M} } } */
> +/* { dg-final { scan-assembler-times {\mrldicr\M} 1 } } */
> --
> 2.21.0.777.g83232e3864
>
>
