On Tue, Dec 03, 2013 at 02:14:17PM -0700, Jeff Law wrote:
> Perhaps split this patch into two parts which can be reviewed
> independently, but go into the tree at the same time. The obvious
> hope would be that Uros or one of the other x86 backend folks could
> chime in on that part.
I posted the i?86 bits separately.
> >--- gcc/ubsan.h.mp 2013-11-27 08:46:28.046629473 +0100
> >+++ gcc/ubsan.h 2013-11-27 08:46:57.578753342 +0100
> >@@ -21,6 +21,12 @@ along with GCC; see the file COPYING3.
> > #ifndef GCC_UBSAN_H
> > #define GCC_UBSAN_H
> >
> >+/* From predict.c. */
> >+#define PROB_VERY_UNLIKELY (REG_BR_PROB_BASE / 2000 - 1)
> >+#define PROB_EVEN (REG_BR_PROB_BASE / 2)
> >+#define PROB_VERY_LIKELY (REG_BR_PROB_BASE - PROB_VERY_UNLIKELY)
> >+#define PROB_ALWAYS (REG_BR_PROB_BASE)
> Seems like this should factor out rather than get duplicated.
I moved all the into predict.h, the users now include predict.h.
> >--- gcc/gimple-fold.c.mp 2013-11-27 08:46:27.979629191 +0100
> >+++ gcc/gimple-fold.c 2013-11-27 08:46:57.556753251 +0100
> >@@ -2660,8 +2660,30 @@ gimple_fold_stmt_to_constant_1 (gimple s
> > tree fn;
> >
> > if (gimple_call_internal_p (stmt))
> >- /* No folding yet for these functions. */
> >- return NULL_TREE;
> >+ {
> >+ enum tree_code subcode = ERROR_MARK;
> >+ switch (gimple_call_internal_fn (stmt))
> >+ {
> >+ case IFN_UBSAN_CHECK_ADD: subcode = PLUS_EXPR; break;
> >+ case IFN_UBSAN_CHECK_SUB: subcode = MINUS_EXPR; break;
> >+ case IFN_UBSAN_CHECK_MUL: subcode = MULT_EXPR; break;
> Minor detail, put the case value and associated codes on separate lines.
>
> case FU:
> code;
> more code
> break;
> case BAR
> blah;
> break;
Done.
> >--- gcc/tree-vrp.c.mp 2013-11-27 08:46:28.043629459 +0100
> >+++ gcc/tree-vrp.c 2013-11-27 08:46:57.570753307 +0100
> >@@ -3757,6 +3757,40 @@ extract_range_basic (value_range_t *vr,
> > break;
> > }
> > }
> >+ else if (is_gimple_call (stmt)
> >+ && gimple_call_internal_p (stmt))
> >+ {
> >+ enum tree_code subcode = ERROR_MARK;
> >+ switch (gimple_call_internal_fn (stmt))
> >+ {
> >+ case IFN_UBSAN_CHECK_ADD: subcode = PLUS_EXPR; break;
> >+ case IFN_UBSAN_CHECK_SUB: subcode = MINUS_EXPR; break;
> >+ case IFN_UBSAN_CHECK_MUL: subcode = MULT_EXPR; break;
> >+ default: break;
> Formatting again.
Done.
> Overall the stuff outside the i386 directory looks pretty good,
> though it needs some minor updates. I'd suggest extracting the i386
> bits and pinging them as a separate patch in the hope that we'll get
> Uros's attention.
Done, I posted splitted version of the patch. Thanks for the review.
Marek