PR/Target 47998

Hi All,

This problem optimize rule missing.
gen_lowpart got invalid operand.

I attached fix patch.

-----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 <ysato@users.sourceforge.jp>
> +
> +	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-----

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 <ysato@users.sourceforge.jp>
> > +
> > +	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") <var_decl 0x7ffff7f93000 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-----

-----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 <ysato@users.sourceforge.jp>
>>>> +
>>>> +	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") <var_decl 0x7ffff7f93000 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-----