On 03/14/2011 10:58 AM, Zdenek Dvorak wrote:
> Hi,
> 
>> diff -u gcc/tree-ssa-loop-ivopts.c gcc/tree-ssa-loop-ivopts.c
>> --- gcc/tree-ssa-loop-ivopts.c	(working copy)
>> +++ gcc/tree-ssa-loop-ivopts.c	(working copy)
>> @@ -1194,6 +1300,7 @@ record_use (struct ivopts_data *data, tr
>>  	    gimple stmt, enum use_type use_type)
>>  {
>>    struct iv_use *use = XCNEW (struct iv_use);
>> +  tree tmp;
>>  
>>    use->id = n_iv_uses (data);
>>    use->type = use_type;
>> @@ -1204,11 +1311,14 @@ record_use (struct ivopts_data *data, tr
>>  
>>    /* To avoid showing ssa name in the dumps, if it was not reset by the
>>       caller.  */
>> +  tmp = iv->ssa_name;
>>    iv->ssa_name = NULL_TREE;
>>  
>>    if (dump_file && (dump_flags & TDF_DETAILS))
>>      dump_use (dump_file, use);
>>  
>> +  iv->ssa_name = tmp;
>> +
>>    VEC_safe_push (iv_use_p, heap, data->iv_uses, use);
>>  
>>    return use;
> 
> this is not ok.  You can get the ssa name from extract_cond_operands.
> 

Ok, I did that now.

>> +  /* Determine if the exit test is formulated in terms of the phi or the
>> +     increment of the use iv.  */
>> +  use_uses_inced_iv
>> +    = gimple_code (SSA_NAME_DEF_STMT (use->iv->ssa_name)) != GIMPLE_PHI;
>> +
>> +  /* Determine if the exit test is before or after the increment of the
>> +     cand.  */
>> +  use_after_cand_inc
>> +    = stmt_after_increment (data->current_loop, cand, use->stmt);
>> +
>> +  /* For now, we only handle these cases.  */
>> +  if (use_after_cand_inc != use_uses_inced_iv)
>> +    return false;
> 
> what is this trying to achieve?  USE_USES_INCED_IV is pretty much meaningless --
> the value of USE does not depend in any way on whether it comes
> directly from
> a PHI node or from some other calculation.

it is trying to allow for

do
  {
    *p = '\0';
    i++; /* use_uses_inced_iv == true */
    p++; /* use_after_cand_inc == true */
    if (!(i < n))
      break;
  }
while (1);

and for

do
  {
    *p = '\0';
    if (!(i < n))
      break;
    i++; /* use_uses_inced_iv == false */
    p++; /* use_after_cand_inc == false */
  }
while (1);

but not for

do
  {
    *p = '\0';
    i++; /* use_uses_inced_iv == true */
    if (!(i < n))
      break;
    p++; /* use_after_cand_inc == false */
  }
while (1);

and not for

do
  {
    *p = '\0';
    p++; /* use_after_cand_inc == true */
    if (!(i < n))
      break;
    i++; /* use_uses_inced_iv == false */
  }
while (1);

In the latter 2 cases, I cannot simply replace i < n with p < base + n.

>> +  /* Calculate bound.  */
>> +  bound = fold_build_plus (PLUS_EXPR, base_ptr,
>> +			   fold_convert (sizetype, use_lt_bound));
>> +  if (bound == NULL_TREE)
>> +    return false;
> 
> How could the result be NULL_TREE?

It can't, indeed. I removed that now.

- Tom