Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Steve Ellcey]

On Thu, 2015-05-14 at 19:22 +0200, Marek Polacek wrote:
> On Thu, May 14, 2015 at 10:07:56AM -0700, Steve Ellcey wrote:
> > Sorry for the noise, it looks like this failure is not related to any
> > recent changes in GCC.  I still get the error in older GCC's so I think
> > something must have changed in glibc.  I will start digging there.
> 
> Yeah -- strict aliasing should be unrelated to the changes I've recently done
> in the C FE.
> 
>   Marek

FYI: I finally found the change that caused this failure, it is a GCC
patch after all.  It starts with Richard's fix to PR middle-end/66110.
I sent some email to glibc but I am not sure (again) if we want to
change GCC or glibc.

https://sourceware.org/ml/libc-alpha/2015-05/msg00258.html

Steve Ellcey
sell...@imgtec.com

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Marek Polacek]

On Thu, May 14, 2015 at 10:07:56AM -0700, Steve Ellcey wrote:
> Sorry for the noise, it looks like this failure is not related to any
> recent changes in GCC.  I still get the error in older GCC's so I think
> something must have changed in glibc.  I will start digging there.

Yeah -- strict aliasing should be unrelated to the changes I've recently done
in the C FE.

Marek

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Steve Ellcey]

On Thu, 2015-05-14 at 09:42 -0700, Steve Ellcey wrote:
> Marek,
> 
> This may be unrelated to your patches for PR 66127 and 66066 but I am
> having a new failure when building the latest glibc with the latest GCC.
> 
> I haven't yet tracked down exactly which patch caused the problem.  Included
> is a cutdown test case from libio/memstream.c in glibc that results in a
> strict-aliasing error.  Is this is something you already know about or have
> seen?
> 
> In the mean time I will try to figure out exactly which patch caused this 
> error
> to trigger.
> 
> Steve Ellcey
> sell...@imgtec.com

Sorry for the noise, it looks like this failure is not related to any
recent changes in GCC.  I still get the error in older GCC's so I think
something must have changed in glibc.  I will start digging there.

Steve Ellcey
sell...@imgtec.com

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Steve Ellcey]

Marek,

This may be unrelated to your patches for PR 66127 and 66066 but I am
having a new failure when building the latest glibc with the latest GCC.

I haven't yet tracked down exactly which patch caused the problem.  Included
is a cutdown test case from libio/memstream.c in glibc that results in a
strict-aliasing error.  Is this is something you already know about or have
seen?

In the mean time I will try to figure out exactly which patch caused this error
to trigger.

Steve Ellcey
sell...@imgtec.com


typedef unsigned int size_t;
extern void *malloc (size_t __size) __attribute__ ((__nothrow__ )) __attribute__
 ((__malloc__)) ;

struct _IO_FILE_plus { void *vtable; };
void *_IO_mem_jumps;

struct _IO_streambuf { };

typedef struct _IO_strfile_
{
  struct _IO_streambuf _sbf;
} _IO_strfile;

struct _IO_FILE_memstream
{
  _IO_strfile _sf;
};

void open_memstream (int bufloc, int sizeloc)
{
  struct locked_FILE
  {
struct _IO_FILE_memstream fp;
  } *new_f;
  new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE));
  ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf)->vtable = &_IO_mem_jumps;
}


x.c: In function 'open_memstream':
x.c:28:12: error: dereferencing type-punned pointer will break strict-aliasing 
rules [-Werror=strict-aliasing]
   ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf)->vtable = &_IO_mem_jumps;
^
cc1: all warnings being treated as errors

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Marek Polacek]

On Wed, May 13, 2015 at 10:46:06PM +, Joseph Myers wrote:
> Yes.  The two patches are OK together, though I think it would be better 

Thanks.

> to add for_int_const checks also in the cases of unary operations, &&, || 
> and ?: (the last three being cases where it's only the evaluated operands 
> that need to fold to INTEGER_CSTs, not any unevaluated operands).  It may 
> well be the case that no folding at present can fold any of those cases to 
> an INTEGER_CST when it shouldn't be one, but I don't think we should rely 
> on that.

Yeah, I couldn't find a case where it would trigger, but I added for_int_const
checks for the other cases all the same.

I'm attaching the patch here for archival purposes.

Bootstrapped/regtested on x86_64-linux, applying to trunk.

2015-05-14  Marek Polacek  

PR c/66066
PR c/66127
* c-common.c (c_fully_fold): Pass false down to c_fully_fold_internal.
(c_fully_fold_internal): Fold C_MAYBE_CONST_EXPRs with
C_MAYBE_CONST_EXPR_INT_OPERANDS set.  Add FOR_INT_CONST argument and
use it.  If FOR_INT_CONST, require that all evaluated operands be
INTEGER_CSTs.

* c-typeck.c (digest_init): Call pedwarn_init with OPT_Wpedantic
rather than with 0.

* gcc.dg/pr14649-1.c: Add -Wpedantic.
* gcc.dg/pr19984.c: Likewise.
* gcc.dg/pr66066-1.c: New test.
* gcc.dg/pr66066-2.c: New test.
* gcc.dg/pr66066-3.c: New test.

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 7e5ac72..31c4c0d 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -315,7 +315,7 @@ const struct fname_var_t fname_vars[] =
 /* Global visibility options.  */
 struct visibility_flags visibility_options;
 
-static tree c_fully_fold_internal (tree expr, bool, bool *, bool *);
+static tree c_fully_fold_internal (tree expr, bool, bool *, bool *, bool);
 static tree check_case_value (location_t, tree);
 static bool check_case_bounds (location_t, tree, tree, tree *, tree *);
 
@@ -1148,7 +1148,7 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const)
   expr = TREE_OPERAND (expr, 0);
 }
   ret = c_fully_fold_internal (expr, in_init, maybe_const,
-  &maybe_const_itself);
+  &maybe_const_itself, false);
   if (eptype)
 ret = fold_convert_loc (loc, eptype, ret);
   *maybe_const &= maybe_const_itself;
@@ -1161,11 +1161,13 @@ c_fully_fold (tree expr, bool in_init, bool 
*maybe_const)
arithmetic overflow (for C90, *MAYBE_CONST_OPERANDS is carried from
both evaluated and unevaluated subexpressions while
*MAYBE_CONST_ITSELF is carried from only evaluated
-   subexpressions).  */
+   subexpressions).  FOR_INT_CONST indicates if EXPR is an expression
+   with integer constant operands, and if any of the operands doesn't
+   get folded to an integer constant, don't fold the expression itself.  */
 
 static tree
 c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
-  bool *maybe_const_itself)
+  bool *maybe_const_itself, bool for_int_const)
 {
   tree ret = expr;
   enum tree_code code = TREE_CODE (expr);
@@ -1209,7 +1211,11 @@ c_fully_fold_internal (tree expr, bool in_init, bool 
*maybe_const_operands,
   if (C_MAYBE_CONST_EXPR_NON_CONST (expr))
*maybe_const_operands = false;
   if (C_MAYBE_CONST_EXPR_INT_OPERANDS (expr))
-   *maybe_const_itself = false;
+   {
+ *maybe_const_itself = false;
+ inner = c_fully_fold_internal (inner, in_init, maybe_const_operands,
+maybe_const_itself, true);
+   }
   if (pre && !in_init)
ret = build2 (COMPOUND_EXPR, TREE_TYPE (expr), pre, inner);
   else
@@ -1259,7 +1265,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool 
*maybe_const_operands,
   op1 = TREE_OPERAND (expr, 1);
   op2 = TREE_OPERAND (expr, 2);
   op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
-  maybe_const_itself);
+  maybe_const_itself, for_int_const);
   STRIP_TYPE_NOPS (op0);
   if (op0 != orig_op0)
ret = build3 (COMPONENT_REF, TREE_TYPE (expr), op0, op1, op2);
@@ -1276,10 +1282,10 @@ c_fully_fold_internal (tree expr, bool in_init, bool 
*maybe_const_operands,
   op2 = TREE_OPERAND (expr, 2);
   op3 = TREE_OPERAND (expr, 3);
   op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
-  maybe_const_itself);
+  maybe_const_itself, for_int_const);
   STRIP_TYPE_NOPS (op0);
   op1 = c_fully_fold_internal (op1, in_init, maybe_const_operands,
-  maybe_const_itself);
+  maybe_const_itself, for_int_const);
   STRIP_TYPE_NOPS (op1);
   op1 = decl_constant_value_for_optimization (op1);
 

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Marek Polacek]

On Thu, May 14, 2015 at 11:48:55AM +0200, Richard Biener wrote:
> On May 14, 2015 10:10:35 AM GMT+02:00, Marek Polacek  
> wrote:
> >On Thu, May 14, 2015 at 09:59:49AM +0200, Richard Biener wrote:
> >> Do the patches allow us to remove the restrictions from the folding
> >patterns?
> >
> >With the c_fully_fold_internal patches there's no need to tweak any
> >match.pd
> >patterns.  So PR66127 + PR66066 are to be solved solely in the C FE. 
> >Is that
> >what you're asking about?
> 
> No, I'm asking whether we can remove the existing guards in match.pd.

Sorry, I don't know which guards exactly you mean (side effects?) so can't
verify, but as Jakub points out, I doubt we can remove anything at this point
because of C++ FE.  (c_fully_fold{,_internal} are never used in the C++ FE.)

Marek

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Jakub Jelinek]

On Thu, May 14, 2015 at 11:48:55AM +0200, Richard Biener wrote:
> On May 14, 2015 10:10:35 AM GMT+02:00, Marek Polacek  
> wrote:
> >On Thu, May 14, 2015 at 09:59:49AM +0200, Richard Biener wrote:
> >> Do the patches allow us to remove the restrictions from the folding
> >patterns?
> >
> >With the c_fully_fold_internal patches there's no need to tweak any
> >match.pd
> >patterns.  So PR66127 + PR66066 are to be solved solely in the C FE. 
> >Is that
> >what you're asking about?
> 
> No, I'm asking whether we can remove the existing guards in match.pd.

There is also the C++ FE...

Jakub

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Richard Biener]

On May 14, 2015 10:10:35 AM GMT+02:00, Marek Polacek  wrote:
>On Thu, May 14, 2015 at 09:59:49AM +0200, Richard Biener wrote:
>> Do the patches allow us to remove the restrictions from the folding
>patterns?
>
>With the c_fully_fold_internal patches there's no need to tweak any
>match.pd
>patterns.  So PR66127 + PR66066 are to be solved solely in the C FE. 
>Is that
>what you're asking about?

No, I'm asking whether we can remove the existing guards in match.pd.

Richard.

>   Marek

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Marek Polacek]

On Thu, May 14, 2015 at 09:59:49AM +0200, Richard Biener wrote:
> Do the patches allow us to remove the restrictions from the folding patterns?

With the c_fully_fold_internal patches there's no need to tweak any match.pd
patterns.  So PR66127 + PR66066 are to be solved solely in the C FE.  Is that
what you're asking about?

Marek

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Richard Biener]

On May 14, 2015 12:46:06 AM GMT+02:00, Joseph Myers  
wrote:
>On Wed, 13 May 2015, Marek Polacek wrote:
>
>> > Rather, how about having an extra argument to c_fully_fold_internal
>(e.g. 
>> > for_int_const) indicating that the folding is of an expression with
>
>> > integer constant operands (so this argument would be true in the
>new case 
>> > of folding the contents of a C_MAYBE_CONST_EXPR with 
>> > C_MAYBE_CONST_EXPR_INT_OPERANDS set)?  In that special case, 
>> > c_fully_fold_internal would only fold the expression itself if all 
>> > evaluated operands folded to INTEGER_CSTs (so if something that
>doesn't 
>> > get folded, such as division by 0, is encountered, then all
>evaluated 
>> > expressions containing it also don't get folded).
>> 
>> Did you mean something like the following?  It is on top of the
>previous
>> c_fully_fold_internal patch; if it makes any sense, I'll squash these
>> into one.
>
>Yes.  The two patches are OK together, though I think it would be
>better 
>to add for_int_const checks also in the cases of unary operations, &&,
>|| 
>and ?: (the last three being cases where it's only the evaluated
>operands 
>that need to fold to INTEGER_CSTs, not any unevaluated operands).  It
>may 
>well be the case that no folding at present can fold any of those cases
>to 
>an INTEGER_CST when it shouldn't be one, but I don't think we should
>rely 
>on that.
>
>> This still doesn't reject enum { A = 0 * (unsigned) (1 / 0) };, but
>note
>> that we don't reject such an enum at present.
>
>It's rejected with -pedantic-errors, but it should indeed be rejected 
>unconditionally like the other cases.
>
>Casts can do some optimization prematurely, but I don't know if that
>has 
>anything to do with the non-rejection here.

Do the patches allow us to remove the restrictions from the folding patterns?

Richard.

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Joseph Myers]

On Wed, 13 May 2015, Marek Polacek wrote:

> > Rather, how about having an extra argument to c_fully_fold_internal (e.g. 
> > for_int_const) indicating that the folding is of an expression with 
> > integer constant operands (so this argument would be true in the new case 
> > of folding the contents of a C_MAYBE_CONST_EXPR with 
> > C_MAYBE_CONST_EXPR_INT_OPERANDS set)?  In that special case, 
> > c_fully_fold_internal would only fold the expression itself if all 
> > evaluated operands folded to INTEGER_CSTs (so if something that doesn't 
> > get folded, such as division by 0, is encountered, then all evaluated 
> > expressions containing it also don't get folded).
> 
> Did you mean something like the following?  It is on top of the previous
> c_fully_fold_internal patch; if it makes any sense, I'll squash these
> into one.

Yes.  The two patches are OK together, though I think it would be better 
to add for_int_const checks also in the cases of unary operations, &&, || 
and ?: (the last three being cases where it's only the evaluated operands 
that need to fold to INTEGER_CSTs, not any unevaluated operands).  It may 
well be the case that no folding at present can fold any of those cases to 
an INTEGER_CST when it shouldn't be one, but I don't think we should rely 
on that.

> This still doesn't reject enum { A = 0 * (unsigned) (1 / 0) };, but note
> that we don't reject such an enum at present.

It's rejected with -pedantic-errors, but it should indeed be rejected 
unconditionally like the other cases.

Casts can do some optimization prematurely, but I don't know if that has 
anything to do with the non-rejection here.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Marek Polacek]

On Wed, May 13, 2015 at 03:06:00PM +, Joseph Myers wrote:
> > It won't handle e.g. 0 * (unsigned) (1 / 0).
> 
> I think this illustrates why handling this in the folding-for-optimization 
> is a mistake.
> 
> For optimization, it's perfectly fine to fold away 0 * (something without 
> side effects), and division by 0 should only be considered to have side 
> effects if language semantic options were specified to that effect (such 
> as using ubsan), which should then have the effect of producing GENERIC 
> that's not a simple division but represents whatever checks are required.
> 
> Rather, how about having an extra argument to c_fully_fold_internal (e.g. 
> for_int_const) indicating that the folding is of an expression with 
> integer constant operands (so this argument would be true in the new case 
> of folding the contents of a C_MAYBE_CONST_EXPR with 
> C_MAYBE_CONST_EXPR_INT_OPERANDS set)?  In that special case, 
> c_fully_fold_internal would only fold the expression itself if all 
> evaluated operands folded to INTEGER_CSTs (so if something that doesn't 
> get folded, such as division by 0, is encountered, then all evaluated 
> expressions containing it also don't get folded).

Did you mean something like the following?  It is on top of the previous
c_fully_fold_internal patch; if it makes any sense, I'll squash these
into one.

With the following, I don't see any regressions in the C dg.exp testsuite.
This still doesn't reject enum { A = 0 * (unsigned) (1 / 0) };, but note
that we don't reject such an enum at present.

Thanks.

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 24200f0..1dc181d 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -315,7 +315,7 @@ const struct fname_var_t fname_vars[] =
 /* Global visibility options.  */
 struct visibility_flags visibility_options;
 
-static tree c_fully_fold_internal (tree expr, bool, bool *, bool *);
+static tree c_fully_fold_internal (tree expr, bool, bool *, bool *, bool);
 static tree check_case_value (location_t, tree);
 static bool check_case_bounds (location_t, tree, tree, tree *, tree *);
 
@@ -1148,7 +1148,7 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const)
   expr = TREE_OPERAND (expr, 0);
 }
   ret = c_fully_fold_internal (expr, in_init, maybe_const,
-  &maybe_const_itself);
+  &maybe_const_itself, false);
   if (eptype)
 ret = fold_convert_loc (loc, eptype, ret);
   *maybe_const &= maybe_const_itself;
@@ -1161,11 +1161,13 @@ c_fully_fold (tree expr, bool in_init, bool 
*maybe_const)
arithmetic overflow (for C90, *MAYBE_CONST_OPERANDS is carried from
both evaluated and unevaluated subexpressions while
*MAYBE_CONST_ITSELF is carried from only evaluated
-   subexpressions).  */
+   subexpressions).  FOR_INT_CONST indicates if EXPR is an expression
+   with integer constant operands, and if any of the operands doesn't
+   get folded to an integer constant, don't fold the expression itself.  */
 
 static tree
 c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
-  bool *maybe_const_itself)
+  bool *maybe_const_itself, bool for_int_const)
 {
   tree ret = expr;
   enum tree_code code = TREE_CODE (expr);
@@ -1212,7 +1214,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool 
*maybe_const_operands,
{
  *maybe_const_itself = false;
  inner = c_fully_fold_internal (inner, in_init, maybe_const_operands,
-maybe_const_itself);
+maybe_const_itself, true);
}
   if (pre && !in_init)
ret = build2 (COMPOUND_EXPR, TREE_TYPE (expr), pre, inner);
@@ -1263,7 +1265,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool 
*maybe_const_operands,
   op1 = TREE_OPERAND (expr, 1);
   op2 = TREE_OPERAND (expr, 2);
   op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
-  maybe_const_itself);
+  maybe_const_itself, for_int_const);
   STRIP_TYPE_NOPS (op0);
   if (op0 != orig_op0)
ret = build3 (COMPONENT_REF, TREE_TYPE (expr), op0, op1, op2);
@@ -1280,10 +1282,10 @@ c_fully_fold_internal (tree expr, bool in_init, bool 
*maybe_const_operands,
   op2 = TREE_OPERAND (expr, 2);
   op3 = TREE_OPERAND (expr, 3);
   op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
-  maybe_const_itself);
+  maybe_const_itself, for_int_const);
   STRIP_TYPE_NOPS (op0);
   op1 = c_fully_fold_internal (op1, in_init, maybe_const_operands,
-  maybe_const_itself);
+  maybe_const_itself, for_int_const);
   STRIP_TYPE_NOPS (op1);
   op1 = decl_constant_value_for_optimization (op1);
   if (op0 != orig_op0 || op1 

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Joseph Myers]

On Wed, 13 May 2015, Marek Polacek wrote:

> On Wed, May 13, 2015 at 03:55:10PM +0200, Jakub Jelinek wrote:
> > On Wed, May 13, 2015 at 03:41:11PM +0200, Marek Polacek wrote:
> > > As discussed in the PR, match.pd happily folds 0 * whatever into 0.  That
> > > is undesirable from the C/C++ FE POV, since it can make us accept invalid
> > > initializers.
> > > 
> > > So fixed in match.pd -- I'd hope there's a better way to do this, but this
> > > seems to work.  There was some fallout, but nothing unexpected or 
> > > surprising.
> > 
> > Will it handle cases 0 * (int) (1 / 0) etc., when perhaps the division by
> > zero isn't immediately the operand of mult, but somewhere deeper?
> 
> It won't handle e.g. 0 * (unsigned) (1 / 0).

I think this illustrates why handling this in the folding-for-optimization 
is a mistake.

For optimization, it's perfectly fine to fold away 0 * (something without 
side effects), and division by 0 should only be considered to have side 
effects if language semantic options were specified to that effect (such 
as using ubsan), which should then have the effect of producing GENERIC 
that's not a simple division but represents whatever checks are required.

Rather, how about having an extra argument to c_fully_fold_internal (e.g. 
for_int_const) indicating that the folding is of an expression with 
integer constant operands (so this argument would be true in the new case 
of folding the contents of a C_MAYBE_CONST_EXPR with 
C_MAYBE_CONST_EXPR_INT_OPERANDS set)?  In that special case, 
c_fully_fold_internal would only fold the expression itself if all 
evaluated operands folded to INTEGER_CSTs (so if something that doesn't 
get folded, such as division by 0, is encountered, then all evaluated 
expressions containing it also don't get folded).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Richard Biener]

On Wed, 13 May 2015, Marek Polacek wrote:

> On Wed, May 13, 2015 at 04:11:15PM +0200, Marek Polacek wrote:
> > I don't know how to reliably fix this :(.
> 
> Except disabling (0 * X) -> 0 completely, that is.

Well, make sure that X has TREE_SIDE_EFFECTS set.  That way we'd fold
to 0, X.

Richard.

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham 
Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Richard Biener]

On Wed, 13 May 2015, Marek Polacek wrote:

> On Wed, May 13, 2015 at 03:55:10PM +0200, Jakub Jelinek wrote:
> > On Wed, May 13, 2015 at 03:41:11PM +0200, Marek Polacek wrote:
> > > As discussed in the PR, match.pd happily folds 0 * whatever into 0.  That
> > > is undesirable from the C/C++ FE POV, since it can make us accept invalid
> > > initializers.
> > > 
> > > So fixed in match.pd -- I'd hope there's a better way to do this, but this
> > > seems to work.  There was some fallout, but nothing unexpected or 
> > > surprising.
> > 
> > Will it handle cases 0 * (int) (1 / 0) etc., when perhaps the division by
> > zero isn't immediately the operand of mult, but somewhere deeper?
> 
> It won't handle e.g. 0 * (unsigned) (1 / 0).
> 
> > Also, can't the divisor be optimized into 0 only later on, so your code
> > would still see !integer_zerop there and fold into 0?
> > Perhaps the answer is that in both cases we'd have simplified those already
> > into a division by zero.
> 
> Yes, it's a dumb attempt.
> 
> I don't know how to reliably fix this :(.  We really want 
> ...

Yeah, I think we can't reliably "avoid" folding away traps like this
so any attempt is useless (and we should simply not do this).

The only way I see is to make all expressions that might trap set
TREE_SIDE_EFFECTS on the expression, so we'd fold it to
a, 0.

Richard.
-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham 
Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Marek Polacek]

On Wed, May 13, 2015 at 04:11:15PM +0200, Marek Polacek wrote:
> I don't know how to reliably fix this :(.

Except disabling (0 * X) -> 0 completely, that is.

Marek

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Marek Polacek]

On Wed, May 13, 2015 at 03:55:10PM +0200, Jakub Jelinek wrote:
> On Wed, May 13, 2015 at 03:41:11PM +0200, Marek Polacek wrote:
> > As discussed in the PR, match.pd happily folds 0 * whatever into 0.  That
> > is undesirable from the C/C++ FE POV, since it can make us accept invalid
> > initializers.
> > 
> > So fixed in match.pd -- I'd hope there's a better way to do this, but this
> > seems to work.  There was some fallout, but nothing unexpected or 
> > surprising.
> 
> Will it handle cases 0 * (int) (1 / 0) etc., when perhaps the division by
> zero isn't immediately the operand of mult, but somewhere deeper?

It won't handle e.g. 0 * (unsigned) (1 / 0).

> Also, can't the divisor be optimized into 0 only later on, so your code
> would still see !integer_zerop there and fold into 0?
> Perhaps the answer is that in both cases we'd have simplified those already
> into a division by zero.

Yes, it's a dumb attempt.

I don't know how to reliably fix this :(.  We really want 
...

Marek

Re: [PATCH] Don't fold away division by zero (PR middle-end/66127)

[Jakub Jelinek]

On Wed, May 13, 2015 at 03:41:11PM +0200, Marek Polacek wrote:
> As discussed in the PR, match.pd happily folds 0 * whatever into 0.  That
> is undesirable from the C/C++ FE POV, since it can make us accept invalid
> initializers.
> 
> So fixed in match.pd -- I'd hope there's a better way to do this, but this
> seems to work.  There was some fallout, but nothing unexpected or surprising.

Will it handle cases 0 * (int) (1 / 0) etc., when perhaps the division by
zero isn't immediately the operand of mult, but somewhere deeper?
Also, can't the divisor be optimized into 0 only later on, so your code
would still see !integer_zerop there and fold into 0?
Perhaps the answer is that in both cases we'd have simplified those already
into a division by zero.

Jakub