Re: [PATCH] Fix ACATS failures in GCC 3.4

[Eric Botcazou <ebotcazou at act-europe dot fr>]

> This patch is OK for mainline and 3.4.

Thanks.

> Without resorting to ASCII art (unless you really want to :-), could you
> explain the sequence of events that lead to the failure.

It's so kindly asked... :-)

Before your patch, this tree is kept untouched by fold

                 MAX_EXPR
             /             \
NOP_EXPR (long)            MINUS_EXPR
    |                      /        \
NOP_EXPR (integer)  NOP_EXPR (long)  1
    |                   |              
VAR_DECL (R18b)     NOP_EXPR (integer)
                        |
                     VAR_DECL (R17b)

After your patch (because of the changes in tree_swap_operands_p), it is 
turned into

                MAX_EXPR
             /             \
     MINUS_EXPR         VAR_DECL (R18b)
    /          \
NOP_EXPR (long) 1
   |              
NOP_EXPR (integer)
   |
VAR_DECL (R17b)

and R18b is unsigned.

> My curiosity is that, unlike comparisons, the signedness of the MIN_EXPR
> and MAX_EXPR operations can/should be determined by their result type, i.e.
> TREE_UNSIGNED (TREE_TYPE (t)).  Comparisons, on the other hand, have
> Boolean-like result types, independent of the comparison being performed.

MAX_EXPRs are RTL-expanded to comparisons between their operands.

> [I'm also a bit confused why RSHIFT is here, but apparently its been
> this way since the current CVS was created.  Shouldn't the type of an
> RSHIFT be the same as the type of its first operand?  The signedness of
> its second operand is significant, but this is also the case for LSHIFT
> or ROTATEs].

I guess it has (had?) something to do with logical/arithmetical, maybe 
overflow conditions when combining RSHIFTs and LSHIFTs.

> Given this difference between MIN/MAX and comparisons, I'm wondering if
> the real bug isn't a missing call to fold_convert somewhere, or the use
> of an operand's type instead of the operator's type.  Fixing this would
> allow the additional optimizations available from STRIP_SIGN_NOPs that
> aren't available with STRIP_NOPs, and as you point out gnat makes heavy
> use of MIN/MAX.

Maybe the real problem comes from the RTL expander for MAX_EXPR.  I'll 
investigate this.

> My apologies for the breakage.

But you didn't break anything!  Your patch only uncovered a latent problem 
and you could not even see it since the ACATS testsuite was not yet 
contributed by that time.

> Given that ACATS isn't yet tested by default on many/most platforms, is
> there a simple C testcase that could be added to the testsuite to prevent 
> these failures in future?

It is tested by default on x86 and I think it's sufficient to spot such a 
kind of problem.

> A test would also help explain the failure.

Ada testcase distilled from c34005a attached.

-- 
Eric Botcazou

WITH SYSTEM; USE SYSTEM;
WITH Ada.Text_IO; Use Ada.Text_IO;

PROCEDURE P IS

     SUBTYPE COMPONENT IS FLOAT;

     FIRST : CONSTANT := 0;
     LAST  : CONSTANT := 100;

     SUBTYPE INDEX IS INTEGER RANGE FIRST .. LAST;

     TYPE PARENT IS ARRAY (INDEX RANGE <>) OF COMPONENT;

     TYPE T IS NEW PARENT (0..0);
     TYPE T2 IS NEW PARENT (0..1);

     X : T;
     X2 : T2;

BEGIN
       X := (0=>1.0);
       X2 := T2(2.0 & X);

       IF X2 /= (2.0, 1.0) then
       put("INCORRECT & (COMPONENT, ARRAY)");
       END IF;

END P;