------------------------------------------------------------------ Sender:Richard Biener <richard.guent...@gmail.com> Sent At:2020 Mar. 20 (Fri.) 18:12 Recipient:bin.cheng <bin.ch...@linux.alibaba.com> Cc:Andrew Pinski <pins...@gmail.com>; GCC Patches <gcc-patches@gcc.gnu.org> Subject:Re: [PATCH PR93674]Avoid introducing IV of enumeral type in case of -fstrict-enums
On Fri, Mar 20, 2020 at 10:27 AM bin.cheng <bin.ch...@linux.alibaba.com> wrote:
>
> ------------------------------------------------------------------
> Sender:Richard Biener <richard.guent...@gmail.com>
> Sent At:2020 Mar. 3 (Tue.) 17:36
> Recipient:Andrew Pinski <pins...@gmail.com>
> Cc:bin.cheng <bin.ch...@linux.alibaba.com>; GCC Patches
> <gcc-patches@gcc.gnu.org>
> Subject:Re: [PATCH PR93674]Avoid introducing IV of enumeral type in case of
> -fstrict-enums
>
>
> On Mon, Mar 2, 2020 at 6:14 PM Andrew Pinski <pins...@gmail.com> wrote:
> >
> > On Mon, Mar 2, 2020 at 1:40 AM Richard Biener
> > <richard.guent...@gmail.com> wrote:
> > >
> > > On Mon, Mar 2, 2020 at 9:07 AM bin.cheng <bin.ch...@linux.alibaba.com>
> > > wrote:
> > > >
> > > > Hi,
> > > > This is a simple fix for PR93674. It adds cand carefully for enumeral
> > > > type iv_use in
> > > > case of -fstrict-enums, it also avoids computing, replacing iv_use with
> > > > the candidate
> > > > so that no IV of enumeral type is introduced with -fstrict-enums option.
> > > >
> > > > Testcase is also added. Bootstrap and test on x86_64. Any comment?
> > >
> > > I think we should avoid enum-typed (or bool-typed) IVs in general, not
> > > just
> > > with -fstrict-enums. That said, the ENUMERAL_TYPE checks should be
> > > !(INTEGER_TYPE || POINTER_TYPE_P) checks.
> >
> > Maybe even check type_has_mode_precision_p or
> > TYPE_MIN_VALUE/TYPE_MAX_VALUE have the same as the min/max for that
> > precision/signedness.
>
> Indeed we don't want non-mode precision INTEGER_TYPE IVs either. I wouldn't
> check TYPE_MIN/MAX_VALUE here though.
>
> Here is the updated patch with more checks. I also removed the
> computation/elimination part for now.
+ if ((TREE_CODE (basetype) != INTEGER_TYPE && !POINTER_TYPE_P (basetype))
+ || !type_has_mode_precision_p (basetype))
+ {
+ if (!CONVERT_EXPR_P (iv->base))
+ return;
+
+ /* Specially handle such iv_use if it's converted from other ones by
+ adding candidate for the source iv. */
+ base = TREE_OPERAND (iv->base, 0);
+ basetype = TREE_TYPE (base);
+ if (!INTEGRAL_TYPE_P (basetype))
+ return;
I think this is too lax - you'll happily add another non-INTEGER_TYPE
or non-mode-precision IV through this handling. If the conversion
is not value-preserving the IV may also be useless (also the step
might see truncation in its type adjustment). With a widening
conversion there's the issue with different wrap-around behavior.
I guess in most cases we'll not be able to use the IV if it is "bogus"
so all this might be harmless.
The original idea is to depend on the condition that the result won't be a
legal IV in
case of widening. For narrowing, it won't be a problem except what you
mentioned
about non-INTEGER_TYPE or non-mode-precision issue.
Was there any reason to handle this special case? Why not simply
do sth like
The reason is to strength reduce (conversion) operation in RHS by introducing
appropriate
candidate.
And right, your proposed code looks better and more general.
+ if ((TREE_CODE (basetype) != INTEGER_TYPE && !POINTER_TYPE_P (basetype)))
+ || !type_has_mode_precision_p (basetype))
+ {
basetype = lang_hooks.type_for_mode (TYPE_MODE (basetype),
TYPE_UNSIGNED (basetype));
tree step = fold_convert (basetype, iv->step);
add_candidate (...);
?
The following also looks bogus:
+ if (basetype == generic_type_for (basetype))
+ step = fold_convert (basetype, step);
don't we add IVs in generic_type_for () and thus the conversion is
always necessary
anyways? (it has the wrong type from the conversion we just stripped)
This is now discarded.
Patch updated and tested on x86_64. Is it OK?
Thanks,
bin
2020-04-09 Bin Cheng <bin.ch...@linux.alibaba.com>
Richard Biener <rguent...@suse.de>
PR tree-optimization/93674
* tree-ssa-loop-ivopts.c (langhooks.h): New include.
(add_iv_candidate_for_use): For iv_use of non integer or pointer type,
or non-mode precision type, add candidate in unsigned type with the
same precision.
gcc/testsuite
2020-04-09 Bin Cheng <bin.ch...@linux.alibaba.com>
PR tree-optimization/93674
* g++.dg/pr93674.C: New test.
0001-Add-unsigned-type-iv_cand-for-iv_use-with-non-mode-p.patch
Description: Binary data
