| Message ID | BANLkTimCEzBM4KAzoOHFL4=39YzLU996-A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, May 13, 2011 at 1:31 PM, Kai Tietz <ktietz70@googlemail.com> wrote: > Hello, > > this patch adds a check to > > ChangeLog > > 2011-05-13 Kai Tietz > > * gimplify.c (gimplify_expr): Make sure operand is boolified. > * tree-cfg.c (verify_gimple_assign_unary): Check for boolean > compatible type > for TRUTH_NOT_EXPR. > > Bootstrapped for x86_64-pc-linux-gnu (multilib). Ok for apply? Ok with .... > Index: tree-cfg.c > =================================================================== > --- tree-cfg.c (revision 173725) > +++ tree-cfg.c (working copy) > @@ -3342,6 +3342,14 @@ > return false; > > case TRUTH_NOT_EXPR: > + if (!useless_type_conversion_p (boolean_type_node, rhs1_type) > + || !useless_type_conversion_p (boolean_type_node, lhs_type)) the 2nd check removed, it's redundant with the common check below /* For the remaining codes assert there is no conversion involved. */ if (!useless_type_conversion_p (lhs_type, rhs1_type)) { error ("non-trivial conversion in unary operation"); debug_generic_expr (lhs_type); debug_generic_expr (rhs1_type); return true; } and ... > + { > + error ("invalid types in truth not"); > + debug_generic_expr (lhs_type); > + debug_generic_expr (rhs1_type); > + return true; > + } please do not fall through here but add a break. Richard. > case NEGATE_EXPR: > case ABS_EXPR: > case BIT_NOT_EXPR: > > Index: gimplify.c > =================================================================== > --- gimplify.c (revision 173726) > +++ gimplify.c (working copy) > @@ -6754,14 +6754,18 @@ > } > > case TRUTH_NOT_EXPR: > - if (TREE_TYPE (*expr_p) != boolean_type_node) > - { > - tree type = TREE_TYPE (*expr_p); > - *expr_p = fold_convert (type, gimple_boolify (*expr_p)); > - ret = GS_OK; > - break; > - } > + { > + tree org_type = TREE_TYPE (*expr_p); > > + *expr_p = gimple_boolify (*expr_p); > + if (org_type != boolean_type_node) > + { > + *expr_p = fold_convert (org_type, *expr_p); > + ret = GS_OK; > + break; > + } > + } > + > ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p, > is_gimple_val, fb_rvalue); > recalculate_side_effects (*expr_p); >
2011/5/13 Richard Guenther <richard.guenther@gmail.com>: > On Fri, May 13, 2011 at 1:31 PM, Kai Tietz <ktietz70@googlemail.com> wrote: >> Hello, >> >> this patch adds a check to >> >> ChangeLog >> >> 2011-05-13 Kai Tietz >> >> * gimplify.c (gimplify_expr): Make sure operand is boolified. >> * tree-cfg.c (verify_gimple_assign_unary): Check for boolean >> compatible type >> for TRUTH_NOT_EXPR. >> >> Bootstrapped for x86_64-pc-linux-gnu (multilib). Ok for apply? > > Ok with .... > >> Index: tree-cfg.c >> =================================================================== >> --- tree-cfg.c (revision 173725) >> +++ tree-cfg.c (working copy) >> @@ -3342,6 +3342,14 @@ >> return false; >> >> case TRUTH_NOT_EXPR: >> + if (!useless_type_conversion_p (boolean_type_node, rhs1_type) >> + || !useless_type_conversion_p (boolean_type_node, lhs_type)) > > the 2nd check removed, it's redundant with the common check below > > /* For the remaining codes assert there is no conversion involved. */ > if (!useless_type_conversion_p (lhs_type, rhs1_type)) > { > error ("non-trivial conversion in unary operation"); > debug_generic_expr (lhs_type); > debug_generic_expr (rhs1_type); > return true; > } > > and ... > >> + { >> + error ("invalid types in truth not"); >> + debug_generic_expr (lhs_type); >> + debug_generic_expr (rhs1_type); >> + return true; >> + } > > please do not fall through here but add a break. > > Richard. > >> case NEGATE_EXPR: >> case ABS_EXPR: >> case BIT_NOT_EXPR: >> >> Index: gimplify.c >> =================================================================== >> --- gimplify.c (revision 173726) >> +++ gimplify.c (working copy) >> @@ -6754,14 +6754,18 @@ >> } >> >> case TRUTH_NOT_EXPR: >> - if (TREE_TYPE (*expr_p) != boolean_type_node) >> - { >> - tree type = TREE_TYPE (*expr_p); >> - *expr_p = fold_convert (type, gimple_boolify (*expr_p)); >> - ret = GS_OK; >> - break; >> - } >> + { >> + tree org_type = TREE_TYPE (*expr_p); >> >> + *expr_p = gimple_boolify (*expr_p); >> + if (org_type != boolean_type_node) >> + { >> + *expr_p = fold_convert (org_type, *expr_p); >> + ret = GS_OK; >> + break; >> + } >> + } >> + >> ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p, >> is_gimple_val, fb_rvalue); >> recalculate_side_effects (*expr_p); >> > Committed at revision 173732 with your suggested adjustments. Thanks, Kai
On Fri, May 13, 2011 at 6:38 AM, Kai Tietz <ktietz70@googlemail.com> wrote: > 2011/5/13 Richard Guenther <richard.guenther@gmail.com>: >> On Fri, May 13, 2011 at 1:31 PM, Kai Tietz <ktietz70@googlemail.com> wrote: >>> Hello, >>> >>> this patch adds a check to >>> >>> ChangeLog >>> >>> 2011-05-13 Kai Tietz >>> >>> * gimplify.c (gimplify_expr): Make sure operand is boolified. >>> * tree-cfg.c (verify_gimple_assign_unary): Check for boolean >>> compatible type >>> for TRUTH_NOT_EXPR. >>> >>> Bootstrapped for x86_64-pc-linux-gnu (multilib). Ok for apply? >> >> Ok with .... >> > > Committed at revision 173732 with your suggested adjustments. > This may have caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48989
On Fri, May 13, 2011 at 4:31 AM, Kai Tietz <ktietz70@googlemail.com> wrote: > > case TRUTH_NOT_EXPR: > + if (!useless_type_conversion_p (boolean_type_node, rhs1_type) > + || !useless_type_conversion_p (boolean_type_node, lhs_type)) In Fortran (and maybe other langauges) there are booleans with different sizes but the same precision. Can you explain how you handle those and why this can't just be a BOOLEAN_TYPE type? Thanks, Andrew Pinski
2011/5/13 Andrew Pinski <pinskia@gmail.com>: > On Fri, May 13, 2011 at 4:31 AM, Kai Tietz <ktietz70@googlemail.com> wrote: >> >> case TRUTH_NOT_EXPR: >> + if (!useless_type_conversion_p (boolean_type_node, rhs1_type) >> + || !useless_type_conversion_p (boolean_type_node, lhs_type)) > > In Fortran (and maybe other langauges) there are booleans with > different sizes but the same precision. > Can you explain how you handle those and why this can't just be a > BOOLEAN_TYPE type? Well, the issue is to be found in gimple_boolify. It is necessary that it is ensured that operands getting boolified on demand. Doing this by default boolean_type_node is used. To verify that, patch converts also for wider-mode BOOLEAN-types back to boolean_type_node type. Before thi no boolification of operands were done for expressions with any boolean type. As we want to make sure that operands getting fixed, too, but gimplifier doesn't know which resulting type it should have, we truncate it back to 1-bit boolean. I assume that the underlying issue is here that some places introduce - via the backdoor - gimple trees, with abitrary boolean-type. IMHO those places should be fixed and use for this gimplication to make sure things getting normalized. Regards, Kai
2011/5/13 Kai Tietz <ktietz70@googlemail.com>: > 2011/5/13 Andrew Pinski <pinskia@gmail.com>: >> On Fri, May 13, 2011 at 4:31 AM, Kai Tietz <ktietz70@googlemail.com> wrote: >>> >>> case TRUTH_NOT_EXPR: >>> + if (!useless_type_conversion_p (boolean_type_node, rhs1_type) >>> + || !useless_type_conversion_p (boolean_type_node, lhs_type)) >> >> In Fortran (and maybe other langauges) there are booleans with >> different sizes but the same precision. >> Can you explain how you handle those and why this can't just be a >> BOOLEAN_TYPE type? > > Well, the issue is to be found in gimple_boolify. It is necessary that > it is ensured that operands getting boolified on demand. Doing this by > default boolean_type_node is used. > To verify that, patch converts also for wider-mode BOOLEAN-types back > to boolean_type_node type. > Before thi no boolification of operands were done for expressions with > any boolean type. As we want to make sure that operands getting fixed, > too, but gimplifier doesn't know which resulting type it should have, > we truncate it back to 1-bit boolean. > I assume that the underlying issue is here that some places introduce > - via the backdoor - gimple trees, with abitrary boolean-type. IMHO > those places should be fixed and use for this gimplication to make > sure things getting normalized. I investigate this a bit more in detail and indeed it is the fortran FE which do here enter in trans-*.c files their own gimple code. And this code doesn't run over the gimplify_expr routines and so it gets in conflict with tree-cfg.c checks. There are two ways to go. A) Adjust fortran's trans-* stuff so that it uses for conditionals and logical-expressions boolean_type consequent (as in some place this type is used, but sometimes artifical boolean_types direct). Or B) We relax tree-cfg checks for logical ops to allow also BOOLEAN types with wider size then 1-bit. Regards, Kai
On Fri, May 13, 2011 at 11:24 AM, Kai Tietz <ktietz70@googlemail.com> wrote: > boolean_types direct). Or B) We relax tree-cfg checks for logical ops > to allow also BOOLEAN types with wider size then 1-bit. The Boolean types which fortran produces are 1bit wide in precision but not always the same width as the C _Bool type. That is TYPE_PRECISION is 1 for those types but TYPE_SIZE is not the same. Thanks, Andrew Pinski
> In Fortran (and maybe other langauges) there are booleans with > different sizes but the same precision. Ada doesn't have a C-like boolean type either. The patches have introduced: FAIL: gnat.dg/lto1.adb (test for excess errors) /home/eric/svn/gcc/gcc/testsuite/gnat.dg/lto1_pkg.adb:23:1: error: type mismatch in binary truth expression boolean boolean boolean D.2419_18 = D.2417_16 || D.2418_17; +===========================GNAT BUG DETECTED==============================+ | 4.7.0 20110513 (experimental) [trunk revision 173737] (i586-suse-linux-gnu) GCC error:| | verify_gimple failed
Index: tree-cfg.c =================================================================== --- tree-cfg.c (revision 173725) +++ tree-cfg.c (working copy) @@ -3342,6 +3342,14 @@ return false; case TRUTH_NOT_EXPR: + if (!useless_type_conversion_p (boolean_type_node, rhs1_type) + || !useless_type_conversion_p (boolean_type_node, lhs_type)) + { + error ("invalid types in truth not"); + debug_generic_expr (lhs_type); + debug_generic_expr (rhs1_type); + return true; + } case NEGATE_EXPR: case ABS_EXPR: case BIT_NOT_EXPR: Index: gimplify.c =================================================================== --- gimplify.c (revision 173726) +++ gimplify.c (working copy) @@ -6754,14 +6754,18 @@ } case TRUTH_NOT_EXPR: - if (TREE_TYPE (*expr_p) != boolean_type_node) - { - tree type = TREE_TYPE (*expr_p); - *expr_p = fold_convert (type, gimple_boolify (*expr_p)); - ret = GS_OK; - break; - } + { + tree org_type = TREE_TYPE (*expr_p); + *expr_p = gimple_boolify (*expr_p); + if (org_type != boolean_type_node) + { + *expr_p = fold_convert (org_type, *expr_p); + ret = GS_OK; + break; + } + } + ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p, is_gimple_val, fb_rvalue); recalculate_side_effects (*expr_p);
Hello, this patch adds a check to ChangeLog 2011-05-13 Kai Tietz * gimplify.c (gimplify_expr): Make sure operand is boolified. * tree-cfg.c (verify_gimple_assign_unary): Check for boolean compatible type for TRUTH_NOT_EXPR. Bootstrapped for x86_64-pc-linux-gnu (multilib). Ok for apply?