On Mon, Mar 28, 2022 at 05:06:00PM -0500, Segher Boessenkool wrote: > Hi! > > On Mon, Mar 28, 2022 at 12:28:04PM -0400, Michael Meissner wrote: > > In looking at PR target/99293, I noticed that the insn "type" attribute is > > incorrect for the vsx_extract_<mode> insns. In particular: > > > > 1) Simple vector register move should be vecmove (alternative 1); > > 2) Xxpermdi should be vecperm (alternative 2); (and) > > 3) Mfvsrld should be mfvsr (alternative 4). > > > > This patch fixes those attributes. > > But the code does not correspond well to the alternatives, even worse > for BE. It would be much clearer (and even possibly correct!) if it > would just use the alternative # in the code, instead of using twenty > different conditions. There are some important cases that have no > alternative right now, like, when op 1 is the same as op 0: it should > have the constraint "0" for op 1 then, and have cost 0. The register > allocator will then hopefully try to make that happen. > > > --- a/gcc/config/rs6000/vsx.md > > +++ b/gcc/config/rs6000/vsx.md > > @@ -3451,7 +3451,7 @@ (define_insn "vsx_extract_<mode>" > > else > > gcc_unreachable (); > > } > > - [(set_attr "type" "veclogical,mfvsr,mfvsr,vecperm") > > + [(set_attr "type" "vecmove,vecperm,mfvsr,mfvsr") > > (set_attr "isa" "*,*,p8v,p9v")]) > > The generated code is one of > no-op > mfvsrd > fmr > xxlor > mfvsrld > xxpermdi > > Which of the 4 alts are meant to correspond to which of those six > possible generated pieces of code?
The mop comment, fmr, and xxlor are all alternative 1 (target is a VSX register, element number is which double-word that that is in the upper 64-bits of the register). The xxpermdi is alternative 2 (target is a VSX register, elment number is which double-word that is in the lower 64-bits of the register). The mfvsrd is alternative 3 (target is a GPR register, element number is which double-word that is in the upper 64-bits of the register). The mfvsrld is alternative 4 (target is a GPR register, elment number is which double-word that is in the lower 64-bits of the register). In terms of this patch, I had originally rewritten the insn as a define_insn_and_split, and changed the move case into just a move to let the later passes just delete moves to the same register. However, it caused some unrelated errors (in the gcc.dg/tree-ssa/builtin-s*printf tests), so I backed out the code to use split to create moves. But it looks like alternative 2 and alternative 4 have the insn attribute "type" mixed up (alternative 2 should be vecperm, alternative 4 should be move from vsr). -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
