[tree-ssa] Grammar for GIMPLE condexprs

[tree-ssa] Grammar for GIMPLE condexprs

[Diego Novillo]

Jason,

The GIMPLE grammar for conditional expressions says:

-----------------------------------------------------------------------------
      relop
              : '<'
              | '<='
              | '>'
              | '>='
              | '=='
              | '!='

      condexpr
              : val
              | val relop val

      val
              : ID
              | CONST
-----------------------------------------------------------------------------

And yet, the gimplifier is emitting things like

-----------------------------------------------------------------------------
  T.225 = a.224 < 0;
  b.226 = (int)b;
  T.227 = b.226 >= 0;
  if (T.225 ^ T.227)
    {
-----------------------------------------------------------------------------

It shouldn't be too hard to change that to

  if ((T.225 ^ T.227) != 0)

Right?  Do you see any problems if I make that change?  A pass
that I'm working on needs to build TRUTH_NOT_EXPR <cond> and it's
failing when I call fold() on the generated tree (because
invert_truthvalue can't deal with VAR_DECLs).


Thanks.  Diego.

Re: [tree-ssa] Grammar for GIMPLE condexprs

[Jason Merrill]

On Tue, 25 Mar 2003 10:32:41 -0500, Diego Novillo <dnovillo@redhat.com> wrote:

> The GIMPLE grammar for conditional expressions says:
>
> -----------------------------------------------------------------------------
>       relop
>               : '<'
>               | '<='
>               | '>'
>               | '>='
>               | '=='
>               | '!='
>
>       condexpr
>               : val
>               | val relop val
>
>       val
>               : ID
>               | CONST
> -----------------------------------------------------------------------------
>
> And yet, the gimplifier is emitting things like
>
> -----------------------------------------------------------------------------
>   T.225 = a.224 < 0;
>   b.226 = (int)b;
>   T.227 = b.226 >= 0;
>   if (T.225 ^ T.227)

is_simple_condexpr currently allows TRUTH_XOR_EXPR, which I assume is the
code in the above tree, rather than the C ^, which is BIT_XOR_EXPR.  This
seems reasonable to me.

> It shouldn't be too hard to change that to
>
>   if ((T.225 ^ T.227) != 0)
>
> Right?  Do you see any problems if I make that change?

Why would you want to suppress just that comparison?  I note that do_jump
doesn't have special handling for TRUTH_AND_EXPR or TRUTH_OR_EXPR, either.

> A pass that I'm working on needs to build TRUTH_NOT_EXPR <cond> and it's
> failing when I call fold() on the generated tree (because
> invert_truthvalue can't deal with VAR_DECLs).

It should.  Perhaps we should try to give boolean gimple temps BOOLEAN_TYPE
even in C.

Jason

Re: [tree-ssa] Grammar for GIMPLE condexprs

[Diego Novillo]

On Tue, 2003-03-25 at 13:18, Jason Merrill wrote:

> >   T.225 = a.224 < 0;
> >   b.226 = (int)b;
> >   T.227 = b.226 >= 0;
> >   if (T.225 ^ T.227)
> 
> is_simple_condexpr currently allows TRUTH_XOR_EXPR, which I assume is the
> code in the above tree, rather than the C ^, which is BIT_XOR_EXPR.  This
> seems reasonable to me.
> 
Yes.  That '^' is TRUTH_XOR_EXPR.  By moving TRUTH_AND_EXPR,
TRUTH_OR_EXPR and TRUTH_XOR_EXPR to is_simple_binary_expr I force the
gimplifier to create another temporary.  It works, but at the expense of
another temporary.  Ugh.

So, I should probably update the documented grammar to note that GIMPLE
also accepts TRUTH_XOR_EXPR, TRUTH_AND_EXPR and TRUTH_OR_EXPR as a
'relop'.
 

> > It shouldn't be too hard to change that to
> >
> >   if ((T.225 ^ T.227) != 0)
> >
> > Right?  Do you see any problems if I make that change?
> 
> Why would you want to suppress just that comparison?  I note that do_jump
> doesn't have special handling for TRUTH_AND_EXPR or TRUTH_OR_EXPR, either.
> 
I'm not supressing comparisons.  This pass registers known values for
predicates.  When processing the THEN_CLAUSE of the if(), we associate
the predicate expression with the constant value 1.  So, if the
predicate is used again inside the THEN_CLAUSE, it gets replaced with
1.  Similarly for the ELSE_CLAUSE.


> > A pass that I'm working on needs to build TRUTH_NOT_EXPR <cond> and it's
> > failing when I call fold() on the generated tree (because
> > invert_truthvalue can't deal with VAR_DECLs).
> 
> It should.  Perhaps we should try to give boolean gimple temps BOOLEAN_TYPE
> even in C.
> 
Would that stop fold() from getting all confused?  If so, I'd rather do
this.


Diego.

Re: [tree-ssa] Grammar for GIMPLE condexprs

[Jason Merrill]

On 25 Mar 2003 13:33:26 -0500, Diego Novillo <dnovillo@redhat.com> wrote:

> On Tue, 2003-03-25 at 13:18, Jason Merrill wrote:
>
>> >   T.225 = a.224 < 0;
>> >   b.226 = (int)b;
>> >   T.227 = b.226 >= 0;
>> >   if (T.225 ^ T.227)
>> 
>> is_simple_condexpr currently allows TRUTH_XOR_EXPR, which I assume is the
>> code in the above tree, rather than the C ^, which is BIT_XOR_EXPR.  This
>> seems reasonable to me.
>> 
> Yes.  That '^' is TRUTH_XOR_EXPR.  By moving TRUTH_AND_EXPR,
> TRUTH_OR_EXPR and TRUTH_XOR_EXPR to is_simple_binary_expr I force the
> gimplifier to create another temporary.  It works, but at the expense of
> another temporary.  Ugh.

Is that really so horrible?  AFAIK, the main reason for allowing
comparisons in the condition operand is so that we can translate them
directly into conditional jumps.  If we can't do that anyway, I don't see
the harm in using a temporary.

> So, I should probably update the documented grammar to note that GIMPLE
> also accepts TRUTH_XOR_EXPR, TRUTH_AND_EXPR and TRUTH_OR_EXPR as a
> 'relop'.

It should certainly match the code.

>> > It shouldn't be too hard to change that to
>> >
>> >   if ((T.225 ^ T.227) != 0)
>> >
>> > Right?  Do you see any problems if I make that change?
>> 
>> Why would you want to suppress just that comparison?  I note that do_jump
>> doesn't have special handling for TRUTH_AND_EXPR or TRUTH_OR_EXPR, either.
>> 
> I'm not supressing comparisons.  This pass registers known values for
> predicates.  When processing the THEN_CLAUSE of the if(), we associate
> the predicate expression with the constant value 1.  So, if the
> predicate is used again inside the THEN_CLAUSE, it gets replaced with
> 1.  Similarly for the ELSE_CLAUSE.

How hard do you look for a matching predicate?  I doubt that you'd see
T.225 ^ T.227 again, though you might see the original expression involving
a and b.

>
>> > A pass that I'm working on needs to build TRUTH_NOT_EXPR <cond> and it's
>> > failing when I call fold() on the generated tree (because
>> > invert_truthvalue can't deal with VAR_DECLs).
>> 
>> It should.  Perhaps we should try to give boolean gimple temps BOOLEAN_TYPE
>> even in C.
>> 
> Would that stop fold() from getting all confused?  If so, I'd rather do
> this.

It would in this case; you can pass anything with BOOLEAN_TYPE to
invert_truthvalue.

Jason

Re: [tree-ssa] Grammar for GIMPLE condexprs

[law]

In message <wvlptofl26j.fsf@prospero.boston.redhat.com>, Jason Merrill writes:
 >Is that really so horrible?  AFAIK, the main reason for allowing
 >comparisons in the condition operand is so that we can translate them
 >directly into conditional jumps.  If we can't do that anyway, I don't see
 >the harm in using a temporary.
Potentially, yes.  We don't want to be creating unnecessary temporaries or
other nodes if it can be easily avoided.

If we look beyond the missing hunks of infrastructure (C++ CFG and real
ssa->normal translator), then the next big thing IMHO is to evaluate
the quality of our gimple translators.

We know there's a significant compile-time and run-time penalty associated
with gimple translation right now.  Avoiding creation of unnecessary nodes
is one aspect of improving the translator.

Now I'm not implying that is Diego's motivation here, but the creation
of unnecessary nodes (of any type) should be avoided if we can do so
easily.

 >>> > It shouldn't be too hard to change that to
 >>> >
 >>> >   if ((T.225 ^ T.227) != 0)
 >>> >
 >>> > Right?  Do you see any problems if I make that change?
 >>> 
 >>> Why would you want to suppress just that comparison?  I note that do_jump
 >>> doesn't have special handling for TRUTH_AND_EXPR or TRUTH_OR_EXPR, either.
 >>> 
 >> I'm not supressing comparisons.  This pass registers known values for
 >> predicates.  When processing the THEN_CLAUSE of the if(), we associate
 >> the predicate expression with the constant value 1.  So, if the
 >> predicate is used again inside the THEN_CLAUSE, it gets replaced with
 >> 1.  Similarly for the ELSE_CLAUSE.
 >
 >How hard do you look for a matching predicate?  I doubt that you'd see
 >T.225 ^ T.227 again, though you might see the original expression involving
 >a and b.
Value numbering will allow us to catch this kind of stuff, even if the
variable names are different.  

Jeff

Re: [tree-ssa] Grammar for GIMPLE condexprs

[Diego Novillo]

On Tue, 2003-03-25 at 13:40, Jason Merrill wrote:
> On 25 Mar 2003 13:33:26 -0500, Diego Novillo <dnovillo@redhat.com> wrote:
> 
> > On Tue, 2003-03-25 at 13:18, Jason Merrill wrote:
> >
> >> >   T.225 = a.224 < 0;
> >> >   b.226 = (int)b;
> >> >   T.227 = b.226 >= 0;
> >> >   if (T.225 ^ T.227)
> >> 
> >> is_simple_condexpr currently allows TRUTH_XOR_EXPR, which I assume is the
> >> code in the above tree, rather than the C ^, which is BIT_XOR_EXPR.  This
> >> seems reasonable to me.
> >> 
> > Yes.  That '^' is TRUTH_XOR_EXPR.  By moving TRUTH_AND_EXPR,
> > TRUTH_OR_EXPR and TRUTH_XOR_EXPR to is_simple_binary_expr I force the
> > gimplifier to create another temporary.  It works, but at the expense of
> > another temporary.  Ugh.
> 
> Is that really so horrible?  AFAIK, the main reason for allowing
> comparisons in the condition operand is so that we can translate them
> directly into conditional jumps.  If we can't do that anyway, I don't see
> the harm in using a temporary.
> 
Well, I can't say that I have done tests on the generated code.  I don't
really know.  Perhaps I should take a look.  We do have a runtime
performance problem when GIMPLE is enabled, so generating more
temporaries may not be a good idea.


> How hard do you look for a matching predicate?  I doubt that you'd see
> T.225 ^ T.227 again, though you might see the original expression involving
> a and b.
> 
We spend almost zero effort.  It comes for free because these are simple
hash table lookups done while we rename the program into SSA.  Mostly,
this will catch things like:

v = foo()
if (v == 3)
  ... uses of v get replaced with 3 here ...

But, no.  I don't expect this to give huge improvements.  It comes under
the category of "hell, we get it for free".


> > Would that stop fold() from getting all confused?  If so, I'd rather do
> > this.
> 
> It would in this case; you can pass anything with BOOLEAN_TYPE to
> invert_truthvalue.
> 
OK.  I will try that.


Thanks.  Diego.