| Message ID | 55638C4E.8040605@acm.org |
|---|---|
| State | New |
| Headers | show |
On 05/25/2015 04:55 PM, Nathan Sidwell wrote: > This patch addresses 66270, another case where may_alias disrupted the > canonical type system. We ICE as TYPE_CANONICALs differ, but comptypes > think they are the same. > > There seems to be a bit of confusion as to whether pointers that differ > only in TYPE_REF_CAN_ALIAS_ALL are the same canonical type or not. > > Firstly, in tree.c build_pointer_type_for_mode, when the pointed-to type > is not its own canonical type, that means the newly constructed pointer > type is (possibly) not canonical either. So we explicitly build a > canonical type with: > > else if (TYPE_CANONICAL (to_type) != to_type) > TYPE_CANONICAL (t) > = build_reference_type_for_mode (TYPE_CANONICAL (to_type), > mode, false); > > But we're passing 'false' in as 'can_alias_all', rather than pass the > value passed into us. Yes, I actually just changed that a month ago because we were hitting this same ICE from a different direction (bug 50800). Since TYPE_CANONICAL (to_type) doesn't have the may_alias attribute, the canonical pointer shouldn't have TRCAA. > That'll make a difference if the caller passed in > true and to_type doesn't have may_alias set. This is inconsistent at > least, because we could sometimes end up with canonical types with > T_R_C_A_A set (to-type is canonical) and sometimes with it not set. It > seems the right solution is to consider T_R_C_A_A as a distinguisher, > thus we should pass can_alias_all to the canonical type builder. Note > that it is ok to pass the possibly modified can_alias_all in, and not > the incoming value, because we only ever modify it to make it true -- > and in that case the same behavior would happen in the canonical type > builder because to_type and TYPE_CANONICAL (to_type) should have the > same may_alias attribute. Hmm, are you seeing a case where TYPE_CANONICAL (to_type) has the may_alias attribute? > Anyway, that's a bit of collateral confusion I fell over investigating. > With that out of the way, we have to teach comptypes that T_R_C_A_A > affects pointer type equality. Hence add such a check to POINTER_TYPE > case there. Similarly, I removed this check for bug 50800. Jason
On 05/25/15 21:18, Jason Merrill wrote: > On 05/25/2015 04:55 PM, Nathan Sidwell wrote: >> else if (TYPE_CANONICAL (to_type) != to_type) >> TYPE_CANONICAL (t) >> = build_reference_type_for_mode (TYPE_CANONICAL (to_type), >> mode, false); >> >> But we're passing 'false' in as 'can_alias_all', rather than pass the >> value passed into us. > > Yes, I actually just changed that a month ago because we were hitting this same > ICE from a different direction (bug 50800). Since TYPE_CANONICAL (to_type) > doesn't have the may_alias attribute, the canonical pointer shouldn't have TRCAA. Ok, so IIUC a canonical pointer to a may_alias type should have TRCAA but a canonical can_alias_all pointer to a non-may_alias type should not have TRCAA? (i.e. one where CAN_ALIAS_ALL was passed true). Or are you saying that no canonical pointers should have TRCAA? > Hmm, are you seeing a case where TYPE_CANONICAL (to_type) has the may_alias > attribute? Yes. This occurs when the newly created TRCAA pointer is to a self-canonical type. The else if (TYPE_CANONICAL (to_type) != to_type) is false, so the newly created pointer is self-canonical too (and has TRCAA). If the canonical type should not have TRCAA we need to change the if condition to: else if (TYPE_CANONICAL (to_type) != to_type || could_alias_all) where COULD_ALIAS_ALL is the incoming CAN_ALIAS_ALL value. Does that make sense? Making that change does stop the ICE I was seeing, but I've not done a full test yet. nathan
On 05/26/2015 03:00 PM, Nathan Sidwell wrote: > Ok, so IIUC a canonical pointer to a may_alias type should have TRCAA > but a canonical can_alias_all pointer to a non-may_alias type should not > have TRCAA? (i.e. one where CAN_ALIAS_ALL was passed true). Or are you > saying that no canonical pointers should have TRCAA? The former: A canonical pointer should have TRCAA if and only if the canonical referent is may_alias. >> Hmm, are you seeing a case where TYPE_CANONICAL (to_type) has the >> may_alias attribute? > > Yes. This occurs when the newly created TRCAA pointer is to a > self-canonical type. Hmm, seems like that's another problem with your testcase: the canonical variant of __m256 should not have may_alias. But the canonical variant of a class or enum type could still have may_alias, so we still need to handle that case. The patch is OK. Jason
On 05/27/15 12:20, Jason Merrill wrote: > On 05/26/2015 03:00 PM, Nathan Sidwell wrote: >> Ok, so IIUC a canonical pointer to a may_alias type should have TRCAA >> but a canonical can_alias_all pointer to a non-may_alias type should not >> have TRCAA? (i.e. one where CAN_ALIAS_ALL was passed true). Or are you >> saying that no canonical pointers should have TRCAA? > > The former: A canonical pointer should have TRCAA if and only if the canonical > referent is may_alias. > >>> Hmm, are you seeing a case where TYPE_CANONICAL (to_type) has the >>> may_alias attribute? >> >> Yes. This occurs when the newly created TRCAA pointer is to a >> self-canonical type. > > Hmm, seems like that's another problem with your testcase: the canonical variant > of __m256 should not have may_alias. But the canonical variant of a class or > enum type could still have may_alias, so we still need to handle that case. I was unclear in my description. The canonical pointer type was being created with TRCAA set because of the incoming true CAN_ALIAS_ALL parameter. That's fixed by the patch, so we're all good now. nathan
2015-05-25 Nathan Sidwell <nathan@acm.org> PR c++/66270 * tree.c (build_pointer_type_for_mode): Pass can_alias_all to canonical type builder. (build_reference_type_for_mode): Likewise. cp/ * typeck.c (structural_comptypes) [POINTER_TYPE]: Compare TYPE_REF_CAN_ALIAS_ALL. testsuite/ * g++.dg/ext/alias-canon3.C: New. Index: cp/typeck.c =================================================================== --- cp/typeck.c (revision 223636) +++ cp/typeck.c (working copy) @@ -1307,6 +1307,8 @@ structural_comptypes (tree t1, tree t2, if (TYPE_MODE (t1) != TYPE_MODE (t2) || !same_type_p (TREE_TYPE (t1), TREE_TYPE (t2))) return false; + if (TYPE_REF_CAN_ALIAS_ALL (t1) != TYPE_REF_CAN_ALIAS_ALL (t2)) + return false; break; case METHOD_TYPE: Index: tree.c =================================================================== --- tree.c (revision 223636) +++ tree.c (working copy) @@ -7759,7 +7759,7 @@ build_pointer_type_for_mode (tree to_typ else if (TYPE_CANONICAL (to_type) != to_type) TYPE_CANONICAL (t) = build_pointer_type_for_mode (TYPE_CANONICAL (to_type), - mode, false); + mode, can_alias_all); /* Lay out the type. This function has many callers that are concerned with expression-construction, and this simplifies them all. */ @@ -7826,7 +7826,7 @@ build_reference_type_for_mode (tree to_t else if (TYPE_CANONICAL (to_type) != to_type) TYPE_CANONICAL (t) = build_reference_type_for_mode (TYPE_CANONICAL (to_type), - mode, false); + mode, can_alias_all); layout_type (t); Index: testsuite/g++.dg/ext/alias-canon3.C =================================================================== --- testsuite/g++.dg/ext/alias-canon3.C (revision 0) +++ testsuite/g++.dg/ext/alias-canon3.C (working copy) @@ -0,0 +1,12 @@ +// { dg-do compile } +// PR c++/66270 + +typedef float __m256 __attribute__ (( __vector_size__(32), __may_alias__ )); +struct A { + __m256 ymm; + const float &f() const; +}; + +const float &A::f() const { + return ymm[1]; +}
This patch addresses 66270, another case where may_alias disrupted the canonical type system. We ICE as TYPE_CANONICALs differ, but comptypes think they are the same. There seems to be a bit of confusion as to whether pointers that differ only in TYPE_REF_CAN_ALIAS_ALL are the same canonical type or not. Firstly, in tree.c build_pointer_type_for_mode, when the pointed-to type is not its own canonical type, that means the newly constructed pointer type is (possibly) not canonical either. So we explicitly build a canonical type with: else if (TYPE_CANONICAL (to_type) != to_type) TYPE_CANONICAL (t) = build_reference_type_for_mode (TYPE_CANONICAL (to_type), mode, false); But we're passing 'false' in as 'can_alias_all', rather than pass the value passed into us. That'll make a difference if the caller passed in true and to_type doesn't have may_alias set. This is inconsistent at least, because we could sometimes end up with canonical types with T_R_C_A_A set (to-type is canonical) and sometimes with it not set. It seems the right solution is to consider T_R_C_A_A as a distinguisher, thus we should pass can_alias_all to the canonical type builder. Note that it is ok to pass the possibly modified can_alias_all in, and not the incoming value, because we only ever modify it to make it true -- and in that case the same behavior would happen in the canonical type builder because to_type and TYPE_CANONICAL (to_type) should have the same may_alias attribute. Anyway, that's a bit of collateral confusion I fell over investigating. With that out of the way, we have to teach comptypes that T_R_C_A_A affects pointer type equality. Hence add such a check to POINTER_TYPE case there. bootstrapped on x86-linux & tested, ok? nathan