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
