Mikael,

I haven't really tried m68k and I can't say I know anything about it but it will only be affected by this issue I am seeing if it generates instructions of the form:
(set (reg:BI ...)
     (<cmp>:BI (reg:SI ...) (const_int ...)))

If you have something like this then as soon as you expand this instruction, cse1 will (from 4.7 onwards at least) optimize this instruction away when it shouldn't.

I can give this backend a try on Tuesday when I get back to work to see if I can reproduce it (Monday is bank holiday over here).

I have a patch for it which seems to work but my tests end up falling apart somewhere else (but that bit might already be my fault).

Cheers,

Paulo Matos


On 04/05/13 11:50, Mikael Pettersson wrote:
On Fri, 3 May 2013 12:49:14 +0000, Paulo Matos <pma...@broadcom.com> wrote:
Hello,

It seems to me there's a bug in 
simplify_const_relational_operation:simplify-rtx.c.
If you set STORE_VALUE_FLAG to -1, if you get to 
simplify_const_relational_operation
  with code: NE, mode: BImode, op0: reg, op1: const_int 0, then you end up in 
line
4717 calling get_mode_bounds.

get_mode_bounds will unfortunately return min:0, max:-1 for BImode and GCC 
proceeds
to compare val which is 0 using:
/* x != y is always true for y out of range.  */
          if (val < mmin || val > mmax)
            return const_true_rtx;

This simplifies the comparison to const_true_rtx in the case STORE_FLAG_VALUE 
is -1.
This seems flawed.

Unless there's some background reason for this to happen this seems like a bug. 
BImode
is a two value mode: 0 or STORE_FLAG_VALUE (according to trunc_int_to_mode), 
therefore
there are really no bounds and these comparisons in 
simplify_const_relational_operation
should take special care if dealing with BImode. Also, having max < min is 
strange at
best and I can imagine it can result in pretty strange behaviour if a developer 
assumes
max >= min, as usual.

I am interested in comments to this piece of code. I am happy to patch
simplify_const_relational_operation if you agree with what I said.

Cheers,

Paulo Matos

I can't comment on the code in question, but the backend for m68k may be 
affected
since it defines STORE_FLAG_VALUE as -1.  Do you have a testcase that would 
cause
wrong code, or a patch to cure the issue?  I'd be happy to do some testing on
m68k-linux.

/Mikael



Reply via email to