On Sun, Oct 18, 2015 at 7:14 PM, Jan Hubicka <hubi...@ucw.cz> wrote: >> >> Adding back the mode check is fine if all types with the same TYPE_CANONICAL >> have the same mode. Otherwise we'd regress here. I thought we do for >> >> Struct x { int i; }; >> Typedef y x __attribute__((packed)); >> >> And then doing >> >> X x; >> Y y; >> X = y; > > Do you have any idea how to turn this into a testcase? I don't think we could > add packed attribute to typedef. Even in > gimple_canonical_types_compatible_p > /* Can't be the same type if they have different mode. */ > if (TYPE_MODE (t1) != TYPE_MODE (t2)) > return false; > (which IMO may be wrong WRT -mavx flags where modes of same types may be > different > in different TUs)
Ok, so the following works: struct x { int i; }; typedef struct x y __attribute__((aligned(1))); void foo (void) { struct x X; y Y; X = Y; } but we use SImode for y as well even though it's alignment is just one byte ... Not sure what happens on strict-align targets for this and not sure how this cannot be _not_ a problem. Consider void bar (struct x); and bar (Y); or using y *Y and X = *Y or bar (*Y). > Therefore I would say that TYPE_CANONICAL determine mode modulo the fact that > incoplete variant of a complete type will have VOIDmode instead of complete > type's mode (during non-LTO). That is why I allow mode changes for casts from > complete to incomplete. Incomplete have VOIDmode, right? > In longer run I think that every query to useless_type_conversion_p that > contains incomplete types is a confused query. useless_type_conversion_p is > about operations on the value and there are no operations for incomplete type > (and function types). I know that ipa-icf-gimple and the following code in > gimplify-stmt checks this frequently: > /* The FEs may end up building ADDR_EXPRs early on a decl with > an incomplete type. Re-build ADDR_EXPRs in canonical form > here. */ > if (!types_compatible_p (TREE_TYPE (op0), TREE_TYPE (TREE_TYPE (expr)))) > *expr_p = build_fold_addr_expr (op0); > Taking address of incomplete type or functions, naturally, makes sense. We > may > want to check something else here, like simply > TREE_TYPE (op0) != TREE_TYPE (TREE_TYPE (expr)) > and once ipa-icf is cleanded up start sanity checking in > usless_type_conversion > that we use it to force equality only on types that do have values. > > We also can trip it when checking TYPE_METHOD_BASETYPE which may be > incomplete. > This is in the code checking useless_type_conversion on functions that I think > are confused querries anyway - we need the ABI matcher, I am looking into > that. Ok, so given we seem to be fine in practive with TYPE_MODE (type) == TYPE_MODE (TYPE_CANONICAL (type)) (whether that's a but or not ...) I'm fine with re-instantiating the mode check for aggregate types. Please do that with Index: gcc/gimple-expr.c =================================================================== --- gcc/gimple-expr.c (revision 228963) +++ gcc/gimple-expr.c (working copy) @@ -89,8 +89,7 @@ useless_type_conversion_p (tree outer_ty /* Changes in machine mode are never useless conversions unless we deal with aggregate types in which case we defer to later checks. */ - if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type) - && !AGGREGATE_TYPE_P (inner_type)) + if (TYPE_MODE (inner_type) != TYPE_MODE (outer_type)) return false; /* If both the inner and outer types are integral types, then the Can we asses equal sizes when modes are non-BLKmode then? Thus @@ -270,10 +269,9 @@ useless_type_conversion_p (tree outer_ty use the types in move operations. */ else if (AGGREGATE_TYPE_P (inner_type) && TREE_CODE (inner_type) == TREE_CODE (outer_type)) - return (!TYPE_SIZE (outer_type) - || (TYPE_SIZE (inner_type) - && operand_equal_p (TYPE_SIZE (inner_type), - TYPE_SIZE (outer_type), 0))); + return (TYPE_MODE (outer_type) != BLKmode + || operand_equal_p (TYPE_SIZE (inner_type), + TYPE_SIZE (outer_type), 0)); else if (TREE_CODE (inner_type) == OFFSET_TYPE && TREE_CODE (outer_type) == OFFSET_TYPE) ? Hoping for VOIDmode incomplete case. Richard. > Honza >> >> Richard. >> >> >> >Honza >> >> >> >> -- >> >> Eric Botcazou >>