https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80153

--- Comment #5 from amker at gcc dot gnu.org ---
(In reply to Richard Biener from comment #4)
> The reason for the tree-affine oddity is that IVO calls
> 
> #0  tree_to_aff_combination (expr=<addr_expr 0x7ffff69c3fe0>, 
>     type=<integer_type 0x7ffff68b73f0>, comb=0x7fffffffd310)
> 
> that is, tree_to_aff_combination with a mismatched expr/type.  For example
> from
> alloc_iv:
> 
> 1174          tree_to_aff_combination (expr, TREE_TYPE (base), &comb);
> 1175          base = fold_convert (TREE_TYPE (base), aff_combination_to_tree
> (&comb));
> 
I think it doesn't matter if we use base's type or expr's type here.  I will
test using consistent types here.

> that's unexpected.  But the problematic case happens where IVO does right:
> 
> Breakpoint 7, tree_to_aff_combination (expr=<nop_expr 0x7ffff69c3300>, 
>     type=<integer_type 0x7ffff68b73f0>, comb=0x7fffffffd4a0)
> 
> but tree_to_aff_combination calls STRIP_NOPS on expr which is (unsigned int)
> "oops!\n" and thus creates the above problematical case itself.
> 
> We can either avoid stripping the nops or deal with the appearant mismatch
> by converting back the elts we add to 'type'.  I think instrumenting to
> see whether we can assert tree_to_aff_combination gets matched types passed
> (so we can eliminate the type arg) would be nice - we certainly can't handle
> all kind of mismatches sanely.
> 
> Then using STRIP_SIGN_NOPS would be safe but IIRC removing sign conversions
> was intentional (though even that might be problematic).  tree-affine was
> really designed for addresses (so type would always be a pointer).
> 
> So sth like the following should better pass bootstrap / test (IVO will
> trigger
> the assert) but it might require adding some "safe" cases to not regress
> code quality (not sure if we have testcases):
> 
> Index: gcc/tree-affine.c
> ===================================================================
> --- gcc/tree-affine.c   (revision 246414)
> +++ gcc/tree-affine.c   (working copy)
> @@ -261,12 +261,21 @@ tree_to_aff_combination (tree expr, tree
>    HOST_WIDE_INT bitpos, bitsize;
>    machine_mode mode;
>    int unsignedp, reversep, volatilep;
> +  tree exptype = TREE_TYPE (expr);
>  
> -  STRIP_NOPS (expr);
> +  gcc_checking_assert (tree_nop_conversion_p (type, exptype)
> +                      && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (exptype)
> +                      && POINTER_TYPE_P (type) == POINTER_TYPE_P (exptype));
> +
> +  STRIP_SIGN_NOPS (expr);
>  
>    code = TREE_CODE (expr);
>    switch (code)
>      {
> +    CASE_CONVERT:
> +      /* Add safe cases.  */
> +      break;
> +
>      case INTEGER_CST:
>        aff_combination_const (comb, type, wi::to_widest (expr));
>        return;

Seems there is an issue that tree-affine lacks ability differentiating between
(unsgined)(pointer + offset) and (unsigned)pointer + (unsigned)offset.

The current behavior of tree_to_aff_combination always folds type conversion
into operands, generating exact the same affines for above two expressions:

{
  type = unsigned int
  offset = 6
  elements = {
    [0] = "oops!\n" * 1,
    [1] = ivtmp.37_10 * 0xffffffffffffffffffffffffffffffffffffffffffffffff
  }
}

While converting affine back to tree, it takes the other way around, always
generating the latter expression: (unsgined)(pointer + offset).  This causes
problem.

IIUC, there are two possible fixes here.  First one is as you mentioned, we
work conservatively and don't fold type conversion into operands (by not
stripping nop).  I suspect this could causes serious code generation
regression.
The second one is the opposite, we always fold type conversion into operands,
by keeping strip_nops.  While converting affine back to tree, we generate
folded expression instead of trying to preserve pointer_plus expression as now.
I prefer the second one, and understand there is concern since tree affine is
used in code generation we could lose pointer arithmetic semantic information
like the pointer_plus expression never overflows/wraps.  But, I think we can
afford this, considering possible code generation regression in the other way. 
I think there shouldn't be fundamental difference in code generation given we
have or don't have the pointer_plus information.  More important, IMHO, tree
affine should (only?) be used when trying to explore as many CSE opportunities
as possible by breaking most association order issues, like in IVOPTs.  So,
customer of tree affine should (at least for most cases) use tree-affine in
code generation only when it finds out there is benefit to do so.  If there is
no benefit, IVOPT wouldn't choose the corresponding candidate.  Lastly, this
only affects type conversion of pointer_plus expressions (IVOPTs only because
it uses unsigned type in order to avoid overflow handling), a customer only
builds pointer type tree affine won't be affected.

Testing patch, let's see if there is any fallout.  Thanks

Reply via email to