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

Reply via email to