Hi David,
On 2020/7/14 22:17, David Edelsohn wrote:
> 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
This patch won't match the instruction with a "clobber (scratch:V4SF)",
it only matches "(clobber (match_scratch:DI 2 "=r"))", I guess you are
replying to the other patch?
"[PATCH 2/2] rs6000: Define define_insn_and_split to split unspec sldi+or to
rldimi"
Thanks for your fix patch! :)
This patch's regression tested pass on Power8-LE, I re-run these cases on
Power8-LE, and confirmed these could pass, what is your platform please?
BTW, TARGET_NO_SF_SUBREG ensured TARGET_POWERPC64 for this
define_insn_and_split.
Thanks.
Xionghu
>
> 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
>>
>>
