This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Switch conversion: support any ax + b transformation (PR tree-optimization/84436).


On Tue, Oct 23, 2018 at 10:37 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 10/22/18 4:25 PM, Jakub Jelinek wrote:
> > On Mon, Oct 22, 2018 at 04:08:53PM +0200, Martin Liška wrote:
> >> Very valid question. I hope as long as I calculate the linear function
> >> values in wide_int (get via wi::to_wide (switch_element)), then it should
> >> overflow in the same way as original tree type arithmetic. I have a test-case with
> >> overflow: gcc/testsuite/gcc.dg/tree-ssa/pr84436-4.c.
> >>
> >> Do you have any {over,under)flowing test-cases that I should add to test-suite?
> >
> > I'm worried that the calculation you emit into the code could invoke UB at
> > runtime, even if there was no UB in the original code, and later GCC passes
> > would optimize with the assumption that UB doesn't occur.
> > E.g. if the multiplication overflows for one or more of the valid values in
> > the switch and then the addition adds a negative value so that the end
> > result is actually representable.
>
> In order to address that I verified that neither of (a * x) and (a * x) + b {over,under}flow
> in case of TYPE_OVERFLOW_UNDEFINED (type) is true.
>
> Hope it's way how to properly make it safe?

Hmm, if the default: case is unreachable maybe.  But I guess Jakub was
suggesting to do the linear function compute in an unsigned type?

+  /* Let's try to find any linear function a.x + y that can apply to

a * x?

+     given values. 'a' can be calculated as follows:

+      tree t = TREE_TYPE (m_index_expr);

so unsigned_type_for (TREE_TYPE ...)

+      tree tmp = make_ssa_name (t);
+      tree value = fold_build2_loc (loc, MULT_EXPR, t,
+                                   wide_int_to_tree (t, coeff_a),
+                                   m_index_expr);
+

+      gsi_insert_before (&gsi, gimple_build_assign (tmp, value),
GSI_SAME_STMT);
+      value = fold_build2_loc (loc, PLUS_EXPR, t,
+                              tmp, wide_int_to_tree (t, coeff_b));
+      tree tmp2 = make_ssa_name (t);
+      gsi_insert_before (&gsi, gimple_build_assign (tmp2, value),
+                        GSI_SAME_STMT);
+      load = gimple_build_assign (name, NOP_EXPR, fold_convert (t, tmp2));

before the unsigned_type_for that NOP_EXPR would be always redundant.

Please also use

  gimple_seq seq = NULL;
  tree tmp = gimple_build (&seq, MULT_EXPR, type, ...);
  tree tmp2 = gimple_build (&seq, PLUS_EXPR, type, ...);
  tree tmp3 = gimple_convert (&seq, TREE_TYPE (m_index_expr), tmp2);
  gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
  load = gimple_build_assign (name, tmp3);

not sure why you need the extra assignment at the end, not enough
context in the patch.

Richard.


> Martin
>
> >
> >       Jakub
> >
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]