Re: Do not compare types in operands_equal_p if OEP_ADDRESS_OF is set

[Jan Hubicka <hubicka at ucw dot cz>]

Hi,

I see, OEP_CONSTANT_ADDRESS_OF is set in:
return operand_equal_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 0), 
                                TREE_CONSTANT (arg0) && TREE_CONSTANT (arg1)    
                                ? OEP_CONSTANT_ADDRESS_OF | OEP_ADDRESS_OF : 0);
so it is not additive to OEP_ADDRESS_OF, I suppose the existing checks for OEP_ADDRESS_OF
in MEM_REF and INDIRECT_REF should also check for OEP_CONSTANT_ADDRESS_OF.  I will sent
separate patch for that.
> > -  if (element_precision (TREE_TYPE (arg0))
> > -      != element_precision (TREE_TYPE (arg1)))
> > -    return 0;
> > +      /* If both types don't have the same precision, then it is not safe
> > +	 to strip NOPs.  */
> > +      if (element_precision (TREE_TYPE (arg0))
> > +	  != element_precision (TREE_TYPE (arg1)))
> > +	return 0;
> 
> It's odd that you move this under the !OEP_ADDRESS_OF case but
> not the STRIP_NOPS itself.

Hmm, I suppose NOP_EXPR should not happen here as it does not have address defined.
I will try to assert there and move the statement around.
> 
> > +    }
> >  
> >    STRIP_NOPS (arg0);
> >    STRIP_NOPS (arg1);
> > @@ -2935,27 +2940,34 @@ operand_equal_p (const_tree arg0, const_
> >  
> >  	case TARGET_MEM_REF:
> >  	case MEM_REF:
> > -	  /* Require equal access sizes, and similar pointer types.
> > -	     We can have incomplete types for array references of
> > -	     variable-sized arrays from the Fortran frontend
> > -	     though.  Also verify the types are compatible.  */
> > -	  if (!((TYPE_SIZE (TREE_TYPE (arg0)) == TYPE_SIZE (TREE_TYPE (arg1))
> > -		   || (TYPE_SIZE (TREE_TYPE (arg0))
> > -		       && TYPE_SIZE (TREE_TYPE (arg1))
> > -		       && operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)),
> > -					   TYPE_SIZE (TREE_TYPE (arg1)), flags)))
> > -		  && types_compatible_p (TREE_TYPE (arg0), TREE_TYPE (arg1))
> > -		  && ((flags & OEP_ADDRESS_OF)
> > -		      || (alias_ptr_types_compatible_p
> > -			    (TREE_TYPE (TREE_OPERAND (arg0, 1)),
> > -			     TREE_TYPE (TREE_OPERAND (arg1, 1)))
> > -			  && (MR_DEPENDENCE_CLIQUE (arg0)
> > -			      == MR_DEPENDENCE_CLIQUE (arg1))
> > -			  && (MR_DEPENDENCE_BASE (arg0)
> > -			      == MR_DEPENDENCE_BASE (arg1))
> > -			  && (TYPE_ALIGN (TREE_TYPE (arg0))
> > -			    == TYPE_ALIGN (TREE_TYPE (arg1)))))))
> > -	    return 0;
> > +	  if (!(flags & OEP_ADDRESS_OF))
> > +	    {
> > +	      /* Require equal access sizes */
> > +	      if (TYPE_SIZE (TREE_TYPE (arg0)) != TYPE_SIZE (TREE_TYPE (arg1))
> > +		  && (!TYPE_SIZE (TREE_TYPE (arg0))
> > +		      || !TYPE_SIZE (TREE_TYPE (arg1))
> > +		      || !operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)),
> > +					   TYPE_SIZE (TREE_TYPE (arg1)),
> > +					   flags & ~OEP_CONSTANT_ADDRESS_OF)))
> 
> so you still pass OEP_ADDRESS_OF ...

I don't because it is tested earlier 
  if (!(flags & OEP_ADDRESS_OF))
> 
> > +		return 0;
> > +	      /* Verify that access happens in similar types.  */
> > +	      if (!types_compatible_p (TREE_TYPE (arg0), TREE_TYPE (arg1)))
> > +		return 0;
> > +	      /* Verify that accesses are TBAA compatible.  */
> > +	      if ((flag_strict_aliasing
> > +		   && !alias_ptr_types_compatible_p
> > +		        (TREE_TYPE (TREE_OPERAND (arg0, 1)),
> > +		         TREE_TYPE (TREE_OPERAND (arg1, 1))))
> > +		  || MR_DEPENDENCE_CLIQUE (arg0)
> > +		     != MR_DEPENDENCE_CLIQUE (arg1)
> > +		  || MR_DEPENDENCE_BASE (arg0)
> > +		     != MR_DEPENDENCE_BASE (arg1))
> > +		return 0;
> > +	    }
> > +	   /* Verify that alignment is compatible.  */
> > +	   if (TYPE_ALIGN (TREE_TYPE (arg0))
> > +	       != TYPE_ALIGN (TREE_TYPE (arg1)))
> > +	      return 0;
> 
> why's compatible alignment required for OEP_ADDRESS_OF?  We only
> look at type alignment on memory references (see get_pointer_alignment
> vs. get_object_alignment).

I actually tested it with the TYPE_ALIGN test in the condtional, too, so I know
it works.  Later I dediced to play safe and that possibly get_pointer_alignment
may want to do that.  Will move it back
to "if (!(flags & OEP_ADDRESS_OF))"

Honza
> 
> >  	  flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF);
> >  	  return (OP_SAME (0) && OP_SAME (1)
> >  		  /* TARGET_MEM_REF require equal extra operands.  */
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)