On Tue, Oct 23, 2018 at 10:37 AM Martin Liška <mli...@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
> >
>
