Re: [C++/66270] another may_alias crash
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
Re: [C++/66270] another may_alias crash
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
Re: [C++/66270] another may_alias crash
On 05/26/15 15:00, Nathan Sidwell wrote: On 05/25/15 21:18, Jason Merrill wrote: 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. Here's a patch implementing that change, When build_pointer_type_for_mode is passed true for CAN_ALIAS_ALL, we force creating a canonical type, continuing to pass false for that pointer's creation. booted & tested on x86-64-linux, ok? nathan 2015-05-25 Nathan Sidwell PR c++/66270 * tree.c (build_pointer_type_for_mode): Canonical type does not inherit can_alias_all. (build_reference_type_for_mode): Likewise. PR c++/66270 * g++.dg/ext/alias-canon3.C: New. 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]; +} Index: tree.c === --- tree.c (revision 223636) +++ tree.c (working copy) @@ -7719,6 +7719,7 @@ build_pointer_type_for_mode (tree to_typ bool can_alias_all) { tree t; + bool could_alias = can_alias_all; if (to_type == error_mark_node) return error_mark_node; @@ -7756,7 +7757,7 @@ build_pointer_type_for_mode (tree to_typ if (TYPE_STRUCTURAL_EQUALITY_P (to_type)) SET_TYPE_STRUCTURAL_EQUALITY (t); - else if (TYPE_CANONICAL (to_type) != to_type) + else if (TYPE_CANONICAL (to_type) != to_type || could_alias) TYPE_CANONICAL (t) = build_pointer_type_for_mode (TYPE_CANONICAL (to_type), mode, false); @@ -7786,6 +7787,7 @@ build_reference_type_for_mode (tree to_t bool can_alias_all) { tree t; + bool could_alias = can_alias_all; if (to_type == error_mark_node) return error_mark_node; @@ -7823,7 +7825,7 @@ build_reference_type_for_mode (tree to_t if (TYPE_STRUCTURAL_EQUALITY_P (to_type)) SET_TYPE_STRUCTURAL_EQUALITY (t); - else if (TYPE_CANONICAL (to_type) != to_type) + else if (TYPE_CANONICAL (to_type) != to_type || could_alias) TYPE_CANONICAL (t) = build_reference_type_for_mode (TYPE_CANONICAL (to_type), mode, false);
Re: [C++/66270] another may_alias crash
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
Re: [C++/66270] another may_alias crash
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