Re: [PATCH] PR/Target 47998
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/17/11 09:31, Yoshinori Sato wrote: > At Wed, 16 Mar 2011 10:22:20 -0600, > Jeff Law wrote: >> > On 03/16/11 09:32, Yoshinori Sato wrote: Hi All, This problem optimize rule missing. gen_lowpart got invalid operand. I attached fix patch. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index ca9398c..9982644 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2011-03-16 Yoshinori Sato + + PR/target 47998 + * config/h8300/h8300.md + (peephole): Add rule. > > It appears that you're restricting operands1 to be a REG/SUBREG, if > that's what your intention is, then changing the operand predicate seems > to be a better solution. > > Furthermore, you'd need to explain why restricting operand1 to be a > REG/SUBREG is the right thing to do. > >> This rtl machied it. >> (const:SI (plus:SI (symbol_ref:SI ("buf") ) >> (const_int 31 [0x1f]))) > >> But gen_lowpart_general not supported this operand. >> So assert test failed. >> I think this rule is inapposite. OK. That's helpful. It seems to me that your change will cause poorer code generation when operand1 is a MEM. I think you can address this problem by changing the "general_operand" predicate to "general_operand_dst". While the name "general_operand_dst" is unfortunate given the proposed usage in the peephole, I think it's the proper predicate. Using that should fix your bug without causing poorer code generation. I think you should fix the other closely related peepholes (there are 3 or 4 which do similar things and appear to all have the same underlying problem allowing (const (plus ...)) then passing that operand to gen_lowpart. Jeff -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJNgju7AAoJEBRtltQi2kC77VAIAIirYQqVC22HsvpXD/03dRmd clu8flmbwATTCpDcH1NqLJVE9rbKFKdyKhPKXtjO3LvhfVT8wtghXeeE1rddzjWQ J64T7rMuB4sKSThlhLTkDUF2rByEijVwWlMsUIIHq5TXIkAaCHri/tFgCXou2pOr +t+jVVjEscQS2x+swVqTOoQecPuwlzCS5iKcoEtvBcJARs2Bsj54sXwcrTaPxM9R uI2U3f7Nh9aHhXYfk6dBjo4RNxud0Rr0giUbEEr2vyYLembJFJ58ghroADChe78p bH1TBxtFJPCSI1umnEgarw/TXAEMfwKaoefmRsLsoNuUyLOiPZvrp/W5op+Q23Q= =/+MT -END PGP SIGNATURE-
Re: [PATCH] PR/Target 47998
At Wed, 16 Mar 2011 10:22:20 -0600, Jeff Law wrote: > > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 03/16/11 09:32, Yoshinori Sato wrote: > > Hi All, > > > > This problem optimize rule missing. > > gen_lowpart got invalid operand. > > > > I attached fix patch. > > > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > > index ca9398c..9982644 100644 > > --- a/gcc/ChangeLog > > +++ b/gcc/ChangeLog > > @@ -1,3 +1,9 @@ > > +2011-03-16 Yoshinori Sato > > + > > + PR/target 47998 > > + * config/h8300/h8300.md > > + (peephole): Add rule. > > It appears that you're restricting operands1 to be a REG/SUBREG, if > that's what your intention is, then changing the operand predicate seems > to be a better solution. > > Furthermore, you'd need to explain why restricting operand1 to be a > REG/SUBREG is the right thing to do. This rtl machied it. (const:SI (plus:SI (symbol_ref:SI ("buf") ) (const_int 31 [0x1f]))) But gen_lowpart_general not supported this operand. So assert test failed. I think this rule is inapposite. > It'd also be helpful to know why this problem does not occur on the > trunk or gcc-4.6 branches, which appear to have the same code as the > gcc-4.5 branch for the peephole. Trunk is not happen in test case. It generate other rtl. > Jeff > -BEGIN PGP SIGNATURE- > Version: GnuPG v1.4.11 (GNU/Linux) > Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ > > iQEcBAEBAgAGBQJNgOO8AAoJEBRtltQi2kC7Od0H/1P3Qp7e8idEu3WLdmHhsA6M > R2KL7+UY1ZzLJpF8+cVjIVRfSTRGc+F8URtqIvaYxP3NJB+ZYiiSVE9jPxegSSDy > /pqx0fLpQach0e4IcPzxFPg/HQkZBnwhW3bkCjxnUARH6hnbbIjFMxfBjJFAZQn+ > QghwaZRutJE6RirbxyqmzEoCzfO76aZ3OJQOxwGYpwsMjoNKFknls0wYXvV6xBBC > +vyamc1hN+/g6X8OH7vfWEP6Q3E9rSrtan+2wCYBZvBXFYWjBbFEfZD94yg5VMbZ > EFoSlmKZCn+c+bsop0T/jsUMBgy05Tv1orbdjsQ0aXKjsGusWlcboo6Is4Inonk= > =Qmwv > -END PGP SIGNATURE- -- Yoshinori Sato
Re: [PATCH] PR/Target 47998
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/16/11 09:32, Yoshinori Sato wrote: > Hi All, > > This problem optimize rule missing. > gen_lowpart got invalid operand. > > I attached fix patch. > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > index ca9398c..9982644 100644 > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,3 +1,9 @@ > +2011-03-16 Yoshinori Sato > + > + PR/target 47998 > + * config/h8300/h8300.md > + (peephole): Add rule. It appears that you're restricting operands1 to be a REG/SUBREG, if that's what your intention is, then changing the operand predicate seems to be a better solution. Furthermore, you'd need to explain why restricting operand1 to be a REG/SUBREG is the right thing to do. It'd also be helpful to know why this problem does not occur on the trunk or gcc-4.6 branches, which appear to have the same code as the gcc-4.5 branch for the peephole. Jeff -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJNgOO8AAoJEBRtltQi2kC7Od0H/1P3Qp7e8idEu3WLdmHhsA6M R2KL7+UY1ZzLJpF8+cVjIVRfSTRGc+F8URtqIvaYxP3NJB+ZYiiSVE9jPxegSSDy /pqx0fLpQach0e4IcPzxFPg/HQkZBnwhW3bkCjxnUARH6hnbbIjFMxfBjJFAZQn+ QghwaZRutJE6RirbxyqmzEoCzfO76aZ3OJQOxwGYpwsMjoNKFknls0wYXvV6xBBC +vyamc1hN+/g6X8OH7vfWEP6Q3E9rSrtan+2wCYBZvBXFYWjBbFEfZD94yg5VMbZ EFoSlmKZCn+c+bsop0T/jsUMBgy05Tv1orbdjsQ0aXKjsGusWlcboo6Is4Inonk= =Qmwv -END PGP SIGNATURE-