On Thu, Mar 23, 2023 at 04:10:22PM +0800, Kewen.Lin wrote: > Hi Mike, > > Thanks for fixing this, some minor comments are inlined below. > > on 2023/3/22 07:53, Michael Meissner wrote: > > The issue with the bug is the power10 load GPR + cmpi -1/0/1 fusion > > optimization generates illegal assembler code. > > > > Ultimately the code was dying because the fusion load + compare -1/0/1 > > patterns > > did not handle the possibility that the load might be prefixed. > > > > The main cause is the constraints for the individual loads in the fusion > > did not > > match the machine. In particular, LWA is a ds format instruction when it is > > unprefixed. The code did not also set the prefixed attribute correctly. > > > > This patch rewrites the genfusion.pl script so that it will have more > > accurate > > constraints for the LWA and LD instructions (which are DS instructions). > > The > > updated genfusion.pl was then run to update fusion.md. Finally, the code > > for > > the "prefixed" attribute is modified so that it considers load + compare > > immediate patterns to be like the normal load insns in checking whether > > operand[1] is a prefixed instruction. > > > > I have tested this patch on a little endian power10 system, on a little > > endian > > power9 system, and a big endian power8 system (both -m32 and -m64 tested on > > BE). There were no regressions, can I check this into the trunk? > > > > The same patch applies to the gcc-12 and gcc-11 branches. Can I check this > > patch into those branches also after a burn-in period? > > > > 2023-03-21 Michael Meissner <meiss...@linux.ibm.com> > > Aaron Sawdey <acsaw...@linux.ibm.com> > > > > gcc/ > > > > PR target/105325 > > * gcc/config/rs6000/genfusion.pl (gen_ld_cmpi_p10): Improve generation > > of the ld and lwa instructions which use the DS encoding instead of D. > > Use the YZ constraint for these loads. Handle prefixed loads better. > > Set the sign_extend attribute as appropriate. > > * gcc/config/rs6000/fusion.md: Regenerate. > > * gcc/config/rs6000/rs6000.md (prefixed attribute): Add fused_load_cmpi > > instructions to the list of instructions that might have a prefixed load > > instruction. > > > > gcc/testsuite/ > > > > PR target/105325 > > * g++.target/powerpc/pr105325.C: New test. > > * gcc.target/powerpc/fusion-p10-ldcmpi.c: Adjust insn counts. > > --- > > gcc/config/rs6000/genfusion.pl | 26 ++++++++++++++++--- > > gcc/config/rs6000/fusion.md | 17 +++++++----- > > gcc/config/rs6000/rs6000.md | 2 +- > > gcc/testsuite/g++.target/powerpc/pr105325.C | 24 +++++++++++++++++ > > .../gcc.target/powerpc/fusion-p10-ldcmpi.c | 4 +-- > > 5 files changed, 59 insertions(+), 14 deletions(-) > > create mode 100644 gcc/testsuite/g++.target/powerpc/pr105325.C > > > > diff --git a/gcc/config/rs6000/genfusion.pl b/gcc/config/rs6000/genfusion.pl > > index e4db352e0ce..4f367cadc52 100755 > > --- a/gcc/config/rs6000/genfusion.pl > > +++ b/gcc/config/rs6000/genfusion.pl > > @@ -56,7 +56,7 @@ sub mode_to_ldst_char > > sub gen_ld_cmpi_p10 > > { > > my ($lmode, $ldst, $clobbermode, $result, $cmpl, $echr, $constpred, > > - $mempred, $ccmode, $np, $extend, $resultmode); > > + $mempred, $ccmode, $np, $extend, $resultmode, $constraint); > > LMODE: foreach $lmode ('DI','SI','HI','QI') { > > $ldst = mode_to_ldst_char($lmode); > > $clobbermode = $lmode; > > @@ -71,21 +71,34 @@ sub gen_ld_cmpi_p10 > > CCMODE: foreach $ccmode ('CC','CCUNS') { > > $np = "NON_PREFIXED_D"; > > $mempred = "non_update_memory_operand"; > > + $constraint = "m"; > > The three assignments on $np $mempred $constraint can be moved > to place (a) (see below) and add one explicit assignment for > $constraint at place (b), since for the condition ccmode eq 'CC', > HI/SI/DI have their own settings (btw QI is skipped), these > assignments for default value can be moved to else arm (for CCUNS).
... > we have broken it into two different arms for SI and DI, this > comment can be removed? ... > > ... and this comment. > I have fixed these issues and reposted the patch as: | Date: Fri, 24 Mar 2023 19:06:35 -0400 | From: Michael Meissner <meiss...@linux.ibm.com> | Subject: [PATCH, V2] PR target/105325, Make load/cmp fusion know about prefixed load | Message-ID: <ZB4s+1RqBNR49tj/@toto.the-meissners.org> -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com