On 9/9/20 4:41 PM, Segher Boessenkool wrote: > Hi! > > On Wed, Sep 09, 2020 at 04:14:37PM -0500, Pat Haugen wrote: >> I noticed that some of the VSR<->GPR move instructions are not typed >> correctly. This patch fixes those instructions so that the scheduler >> treats them with the correct latency. > >> --- a/gcc/config/rs6000/rs6000.md >> +++ b/gcc/config/rs6000/rs6000.md >> @@ -5483,7 +5483,7 @@ (define_insn "lfiwzx" >> lxsiwzx %x0,%y1 >> mtvsrwz %x0,%1 >> xxextractuw %x0,%x1,4" >> - [(set_attr "type" "fpload,fpload,mftgpr,vecexts") >> + [(set_attr "type" "fpload,fpload,mffgpr,vecexts") >> (set_attr "isa" "*,p8v,p8v,p9v")]) > > Can we rename mftgpr/mffgpr globally? Maybe even as mfvsr and mtvsr, > because that is what is actually modeled here? Such names will make it > much harder to get confused and use the wrong type, too :-) >
Those types were originally created for the mffgpr/mftgpr Power6 instructions. But since it appears we no longer generate those insns I totally agree with doing a global change as you suggest to make things clearer. Would you like that as a separate patch or is it fine to include in this one? >> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md >> index 54da54c43dc..3a5cf896da8 100644 >> --- a/gcc/config/rs6000/vsx.md >> +++ b/gcc/config/rs6000/vsx.md >> @@ -2885,7 +2885,7 @@ (define_insn "vsx_concat_<mode>" >> else >> gcc_unreachable (); >> } >> - [(set_attr "type" "vecperm")]) >> + [(set_attr "type" "vecperm,vecmove")]) > > mtvsrdd is a mtvsr, sorry, mffgpr just the same? It isn't vecmove? > >> @@ -4440,7 +4440,7 @@ (define_insn "vsx_splat_<mode>_reg" >> "@ >> xxpermdi %x0,%x1,%x1,0 >> mtvsrdd %x0,%1,%1" >> - [(set_attr "type" "vecperm")]) >> + [(set_attr "type" "vecperm,vecmove")]) > > Same here. mtvsrdd dispatches as a vector op, so requires a super-slice. As opposed to the others which just require a single execution slice for Power9. -Pat
