Re: [Gcc4cli-devel] Several small patches

[Thierry L. <thi...@in...>]

Hi Gabriele,

Thanks for your quick answer. My remarks below...

Gabriele Svelto a écrit :
>  Hi Thierry.
>
> 2010/3/8 Thierry Lafage <thi...@in...>:
>   
>> cil-lower.c.patch modifies the pass gate in order to activate this pass only
>> when -ftree_vectorize option is activated (which is the case in -O3)
>>     
> Unfortunately you cannot do this as the users can explicitly use
> vector intrinsics in the code even if -O3 is not specified. Under
> those conditions you will find vector operations even if you are
> compiling with -O0, it happens quite often in the testsuite BTW.
>   
Ok, sorry, I misunderstood the fixme "should lower only if vector types 
are handled in this funtion."
I simply wanted, for easier debug, to ensure that the execution of this 
pass could be controlled (ie. skipped by using a command-line argument). 
But it's probably useless.

>> and simp-cond.patch is a simplification of the simp-cond pass which also
>> makes it more complete (adds support for brtrue/brfalse and for floating
>> point comparisons in conditional branches + add a command-line option which
>> is activated in -O1 + fixes missing brtrue/brfalse as being conditional
>> branch instructions). Note that this pass either does nothing, or change a
>> branch condition instruction into another one (inverts the condition) and
>> removes the following unconditional branch. This means that the stack is not
>> changed at all, so I removed all the stuff that was recomputing the stack at
>> every statement. Moreover, I didn't understand the test on the conditional
>> branch operand type (integer or pointer) in order to change the conditional
>> br instruction: I've removed it and tested on a real program, and it seems
>> to be ok.
>>     
>
> You cannot remove the integer / pointer test from that pass because
> for floating-point numbers those operations cannot be changed. For
> example if 'x' and 'y' are two floating-point NaNs the result from any
> comparison will always be unordered, i.e. x > y and x <= y will both
> evaluate to false hence you cannot invert the comparison. That's why I
> left out floating-point comparisons and that's why I compute the
> stack: I need to know on what types the comparison is working on. As
> for brtrue and brfalse I don't remember why I have left them out. They
> are different from the other instructions in that they pop off the
> stack a single value and work only on native integers while other
> comparisons pop two values which can be pretty much anything. I might
> have left them out because of that but I don't remember exactly so you
> might give them a try anyway. A full testsuite will give you the
> answer anyway, if there are problems tied to swapping brtrue/brfalse
> they will probably manifest themselves there.
>
>  Gabriele
>   

Ok, for this one, I've been very naive. Sorry. It seems to me that it 
has been a fast copy/paste of a generic pass...
For true/false, it happens that conditions on floating point values may 
be computed by first using a comparison, and then a brtrue or brfalse. 
So inverting the branch may lead to the same problems as with branches 
with FP operands. In this particular case, we could look up at the 
operands of the compare (if any) and invert the branch (or better: the 
compare itself?) only if they are integer or pointers.
For the FP general case, wouldn't it be possible to use *.un (or not) 
versions of the branches? I'll make tests to check.
However, concerning stack computation, since conditional branch 
inversion does not change the stack at all, are the 
"cil_stack_after_stmt (stack, stmt);" and "FOR_EACH_EDGE(...) { 
cil_set_stack_for_bb (...); }" useful?
I understand these statements as being a stack re-compute, isn't it?

Regards,
Thierry.