[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2026-03-14 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

--- Comment #21 from GCC Commits  ---
The releases/gcc-14 branch has been updated by Jakub Jelinek
:

https://gcc.gnu.org/g:68205d83cbaa3dfbab96885e3ab274e75814c2db

commit r14-12372-g68205d83cbaa3dfbab96885e3ab274e75814c2db
Author: Jakub Jelinek 
Date:   Tue Nov 25 10:06:46 2025 +0100

alias: Fix up BITINT_TYPE and non-standard INTEGER_TYPE alias handling
[PR122624]

The testcase in the PR is miscompiled on aarch64 with
--param=ggc-min-expand=0 --param=ggc-min-heapsize=0 -O2
(not including it in the testsuite because it is too much of
a lottery).

Anyway, the problem is that the testcase only uses unsigned _BitInt(66)
and never uses _BitInt(66), get_alias_set remembers alias set for
ARRAY_TYPE (of its element type in ARRAY_TYPE's TYPE_ALIAS_SET),
c_common_get_alias_set does not remember in TYPE_ALIAS_SET alias of
unsigned types and instead asks for get_alias_set of corresponding
signed type and that creates a new alias set for each new canonical
type.
So, in this case, when being asked about get_alias_set on ARRAY_TYPE
unsigned _BitInt(66) [N], it recurses down to c_common_get_alias_set
which asks for alias set of at that point newly created signed type
_BitInt(66), new alias set is created for it, remembered in that
signed _BitInt(66) TYPE_ALIAS_SET, not remembered in unsigned _BitInt(66)
and remembered in ARRAY_TYPE's TYPE_ALIAS_SET.
Next a GC collection comes, signed _BitInt(66) is not used anywhere in
any reachable from GC roots, so it is removed.
Later on we ask alias oracle whether the above mentioned ARRAY_TYPE
can for TBAA alias pointer dereference with the same unsigned _BitInt(66)
type.  For the ARRAY_TYPE, we have the above created alias set remembered
in TYPE_ALIAS_SET, so that is what we use, but for the unsigned _BitInt(66)
we don't, so create a new signed _BitInt(66), create a new alias set for it
and that is what is returned, so we have to distinct alias sets and return
that they can't alias.
Now, for standard INTEGER_TYPEs this isn't a problem, because both the
signed and unsigned variants of those types are always reachable from GTY
roots.  For BITINT_TYPE (or build_nonstandard_integer_type built types)
that isn't the case.  I'm not convinced we need to fix it for
build_nonstandard_integer_type built INTEGER_TYPEs though, for bit-fields
their address can't be taken in C/C++, but for BITINT_TYPE this clearly
is a problem.

So, the following patch solves it by
1) remembering the alias set we got from get_alias_set on the signed
   _BitInt(N) type in the unsigned _BitInt(N) type
2) returning -1 for unsigned _BitInt(1), because there is no corresponding
   signed _BitInt type and so we can handle it normally
3) so that the signed _BitInt(N) type isn't later GC collected and later
   readded with a new alias set incompatible with the still reachable
   unsigned _BitInt(N) type, the patch for signed _BitInt(N) types checks
   if corresponding unsigned _BitInt(N) type doesn't already have
   TYPE_ALIAS_SET_KNOWN_P, in that case it remembers and returns that;
   in order to avoid infinite recursion, it doesn't call get_alias_set
   on the unsigned _BitInt(N) type though
4) while type_hash_canon remembers in the type_hash_table both the hash
   and the type, so what exactly we use as the hash isn't that important,
   I think using type_hash_canon_hash for BITINT_TYPEs is actually better
over
   hasing TYPE_MAX_VALUE, because especially for larger BITINT_TYPEs
   TYPE_MAX_VALUE can have lots of HWIs in the number, for
   type_hash_canon_hash hashes for BITINT_TYPEs only
   i) TREE_CODE (i.e. BITINT_TYPE)
   ii) TYPE_STRUCTURAL_EQUALITY_P flag (likely false)
   iii) TYPE_PRECISION
   iv) TYPE_UNSIGNED
   so 3 ints and one flag, while the old way can hash one HWI up to
   1024 HWIs; note it is also more consistent with most other
   type_hash_canon calls, except for build_nonstandard_integer_type; for
   some reason changing that one to use also type_hash_canon_hash doesn't
   work, causes tons of ICEs

2025-11-25  Jakub Jelinek  

PR middle-end/122624
* c-common.cc (c_common_get_alias_set): Fix up handling of
BITINT_TYPEs.  For unsigned _BitInt(1) always return -1.  For other
unsigned types set TYPE_ALIAS_SET to get_alias_set of corresponding
signed type and return that.  For signed types check if
corresponding
unsigned type has TYPE_ALIAS_SET_KNOWN_P and if so copy its
TYPE_ALIAS_SET and return that.

(cherry picked from commit 5836d9322a2adb0b6d1a5d576fb5ceb9569009b2)

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-12-08 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

Jakub Jelinek  changed:

   What|Removed |Added

   Target Milestone|--- |15.3
 Resolution|--- |FIXED
 Status|ASSIGNED|RESOLVED

--- Comment #20 from Jakub Jelinek  ---
Fixed also for 15.3+.

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-12-07 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

--- Comment #19 from GCC Commits  ---
The releases/gcc-15 branch has been updated by Jakub Jelinek
:

https://gcc.gnu.org/g:a4d224859d0b92c5141a84fa12aa7eb1c1818997

commit r15-10580-ga4d224859d0b92c5141a84fa12aa7eb1c1818997
Author: Jakub Jelinek 
Date:   Tue Nov 25 10:06:46 2025 +0100

alias: Fix up BITINT_TYPE and non-standard INTEGER_TYPE alias handling
[PR122624]

The testcase in the PR is miscompiled on aarch64 with
--param=ggc-min-expand=0 --param=ggc-min-heapsize=0 -O2
(not including it in the testsuite because it is too much of
a lottery).

Anyway, the problem is that the testcase only uses unsigned _BitInt(66)
and never uses _BitInt(66), get_alias_set remembers alias set for
ARRAY_TYPE (of its element type in ARRAY_TYPE's TYPE_ALIAS_SET),
c_common_get_alias_set does not remember in TYPE_ALIAS_SET alias of
unsigned types and instead asks for get_alias_set of corresponding
signed type and that creates a new alias set for each new canonical
type.
So, in this case, when being asked about get_alias_set on ARRAY_TYPE
unsigned _BitInt(66) [N], it recurses down to c_common_get_alias_set
which asks for alias set of at that point newly created signed type
_BitInt(66), new alias set is created for it, remembered in that
signed _BitInt(66) TYPE_ALIAS_SET, not remembered in unsigned _BitInt(66)
and remembered in ARRAY_TYPE's TYPE_ALIAS_SET.
Next a GC collection comes, signed _BitInt(66) is not used anywhere in
any reachable from GC roots, so it is removed.
Later on we ask alias oracle whether the above mentioned ARRAY_TYPE
can for TBAA alias pointer dereference with the same unsigned _BitInt(66)
type.  For the ARRAY_TYPE, we have the above created alias set remembered
in TYPE_ALIAS_SET, so that is what we use, but for the unsigned _BitInt(66)
we don't, so create a new signed _BitInt(66), create a new alias set for it
and that is what is returned, so we have to distinct alias sets and return
that they can't alias.
Now, for standard INTEGER_TYPEs this isn't a problem, because both the
signed and unsigned variants of those types are always reachable from GTY
roots.  For BITINT_TYPE (or build_nonstandard_integer_type built types)
that isn't the case.  I'm not convinced we need to fix it for
build_nonstandard_integer_type built INTEGER_TYPEs though, for bit-fields
their address can't be taken in C/C++, but for BITINT_TYPE this clearly
is a problem.

So, the following patch solves it by
1) remembering the alias set we got from get_alias_set on the signed
   _BitInt(N) type in the unsigned _BitInt(N) type
2) returning -1 for unsigned _BitInt(1), because there is no corresponding
   signed _BitInt type and so we can handle it normally
3) so that the signed _BitInt(N) type isn't later GC collected and later
   readded with a new alias set incompatible with the still reachable
   unsigned _BitInt(N) type, the patch for signed _BitInt(N) types checks
   if corresponding unsigned _BitInt(N) type doesn't already have
   TYPE_ALIAS_SET_KNOWN_P, in that case it remembers and returns that;
   in order to avoid infinite recursion, it doesn't call get_alias_set
   on the unsigned _BitInt(N) type though
4) while type_hash_canon remembers in the type_hash_table both the hash
   and the type, so what exactly we use as the hash isn't that important,
   I think using type_hash_canon_hash for BITINT_TYPEs is actually better
over
   hasing TYPE_MAX_VALUE, because especially for larger BITINT_TYPEs
   TYPE_MAX_VALUE can have lots of HWIs in the number, for
   type_hash_canon_hash hashes for BITINT_TYPEs only
   i) TREE_CODE (i.e. BITINT_TYPE)
   ii) TYPE_STRUCTURAL_EQUALITY_P flag (likely false)
   iii) TYPE_PRECISION
   iv) TYPE_UNSIGNED
   so 3 ints and one flag, while the old way can hash one HWI up to
   1024 HWIs; note it is also more consistent with most other
   type_hash_canon calls, except for build_nonstandard_integer_type; for
   some reason changing that one to use also type_hash_canon_hash doesn't
   work, causes tons of ICEs

2025-11-25  Jakub Jelinek  

PR middle-end/122624
* c-common.cc (c_common_get_alias_set): Fix up handling of
BITINT_TYPEs.  For unsigned _BitInt(1) always return -1.  For other
unsigned types set TYPE_ALIAS_SET to get_alias_set of corresponding
signed type and return that.  For signed types check if
corresponding
unsigned type has TYPE_ALIAS_SET_KNOWN_P and if so copy its
TYPE_ALIAS_SET and return that.

(cherry picked from commit 5836d9322a2adb0b6d1a5d576fb5ceb9569009b2)

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-11-25 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

Jakub Jelinek  changed:

   What|Removed |Added

   Priority|P1  |P2

--- Comment #18 from Jakub Jelinek  ---
Fixed on the trunk so far.

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-11-25 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

--- Comment #17 from GCC Commits  ---
The master branch has been updated by Jakub Jelinek :

https://gcc.gnu.org/g:5836d9322a2adb0b6d1a5d576fb5ceb9569009b2

commit r16-5574-g5836d9322a2adb0b6d1a5d576fb5ceb9569009b2
Author: Jakub Jelinek 
Date:   Tue Nov 25 10:06:46 2025 +0100

alias: Fix up BITINT_TYPE and non-standard INTEGER_TYPE alias handling
[PR122624]

The testcase in the PR is miscompiled on aarch64 with
--param=ggc-min-expand=0 --param=ggc-min-heapsize=0 -O2
(not including it in the testsuite because it is too much of
a lottery).

Anyway, the problem is that the testcase only uses unsigned _BitInt(66)
and never uses _BitInt(66), get_alias_set remembers alias set for
ARRAY_TYPE (of its element type in ARRAY_TYPE's TYPE_ALIAS_SET),
c_common_get_alias_set does not remember in TYPE_ALIAS_SET alias of
unsigned types and instead asks for get_alias_set of corresponding
signed type and that creates a new alias set for each new canonical
type.
So, in this case, when being asked about get_alias_set on ARRAY_TYPE
unsigned _BitInt(66) [N], it recurses down to c_common_get_alias_set
which asks for alias set of at that point newly created signed type
_BitInt(66), new alias set is created for it, remembered in that
signed _BitInt(66) TYPE_ALIAS_SET, not remembered in unsigned _BitInt(66)
and remembered in ARRAY_TYPE's TYPE_ALIAS_SET.
Next a GC collection comes, signed _BitInt(66) is not used anywhere in
any reachable from GC roots, so it is removed.
Later on we ask alias oracle whether the above mentioned ARRAY_TYPE
can for TBAA alias pointer dereference with the same unsigned _BitInt(66)
type.  For the ARRAY_TYPE, we have the above created alias set remembered
in TYPE_ALIAS_SET, so that is what we use, but for the unsigned _BitInt(66)
we don't, so create a new signed _BitInt(66), create a new alias set for it
and that is what is returned, so we have to distinct alias sets and return
that they can't alias.
Now, for standard INTEGER_TYPEs this isn't a problem, because both the
signed and unsigned variants of those types are always reachable from GTY
roots.  For BITINT_TYPE (or build_nonstandard_integer_type built types)
that isn't the case.  I'm not convinced we need to fix it for
build_nonstandard_integer_type built INTEGER_TYPEs though, for bit-fields
their address can't be taken in C/C++, but for BITINT_TYPE this clearly
is a problem.

So, the following patch solves it by
1) remembering the alias set we got from get_alias_set on the signed
   _BitInt(N) type in the unsigned _BitInt(N) type
2) returning -1 for unsigned _BitInt(1), because there is no corresponding
   signed _BitInt type and so we can handle it normally
3) so that the signed _BitInt(N) type isn't later GC collected and later
   readded with a new alias set incompatible with the still reachable
   unsigned _BitInt(N) type, the patch for signed _BitInt(N) types checks
   if corresponding unsigned _BitInt(N) type doesn't already have
   TYPE_ALIAS_SET_KNOWN_P, in that case it remembers and returns that;
   in order to avoid infinite recursion, it doesn't call get_alias_set
   on the unsigned _BitInt(N) type though
4) while type_hash_canon remembers in the type_hash_table both the hash
   and the type, so what exactly we use as the hash isn't that important,
   I think using type_hash_canon_hash for BITINT_TYPEs is actually better
over
   hasing TYPE_MAX_VALUE, because especially for larger BITINT_TYPEs
   TYPE_MAX_VALUE can have lots of HWIs in the number, for
   type_hash_canon_hash hashes for BITINT_TYPEs only
   i) TREE_CODE (i.e. BITINT_TYPE)
   ii) TYPE_STRUCTURAL_EQUALITY_P flag (likely false)
   iii) TYPE_PRECISION
   iv) TYPE_UNSIGNED
   so 3 ints and one flag, while the old way can hash one HWI up to
   1024 HWIs; note it is also more consistent with most other
   type_hash_canon calls, except for build_nonstandard_integer_type; for
   some reason changing that one to use also type_hash_canon_hash doesn't
   work, causes tons of ICEs

This version of the patch handles INTEGER_TYPEs the same as BITINT_TYPE.

2025-11-25  Jakub Jelinek  

PR middle-end/122624
* tree.cc (build_bitint_type): Use type_hash_canon_hash.

* c-common.cc (c_common_get_alias_set): Fix up handling of
BITINT_TYPE
and non-standard INTEGER_TYPEs.  For unsigned _BitInt(1) always
return
-1.  For other unsigned types set TYPE_ALIAS_SET to get_alias_set
of
corresponding signed type and return that.  For signed types check
if
corresponding unsigned type has TYPE_ALIAS_SET_KNOWN_P and if so
copy
its TYPE_ALIAS_SET and return that.

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-11-24 Thread jsm28 at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

--- Comment #16 from Joseph S. Myers  ---
Note incidentally that there's a proposal to add signed _BitInt(1) to C2y that
will be going for an online vote in early December.

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-11-22 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

Jakub Jelinek  changed:

   What|Removed |Added

   Assignee|unassigned at gcc dot gnu.org  |jakub at gcc dot gnu.org
 Status|NEW |ASSIGNED

--- Comment #15 from Jakub Jelinek  ---
Created attachment 62882
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=62882&action=edit
gcc16-pr122624.patch

Untested fix.

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-11-22 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

--- Comment #14 from Jakub Jelinek  ---
My patch doesn't help (but guess it is desirable anyway).
There seems to be 11 distinct _BitInt(66) signed BITINT_TYPEs created during
the --param=ggc-min-expand=0 --param=ggc-min-heapsize=0 -O2 compilation (and
new_alias_set is called on these whenever we ask for alias info of those).
The question is why during the
BS_VAR_11D.4689 = *.LC1D.4701;
vs.
_1 = MEM[(unsigned _BitInt(66) *)_15];
TBAA comparison alias oracle returns that the two can't alias.
BS_VAR_11 has ARRAY_TYPE and that ARRAY_TYPE has recorded TYPE_ALIAS_SET of 4,
but
the TARGET_MEM_REF has just POINTER_TYPE to BITINT_TYPE and that BITINT_TYPE
doesn't have TYPE_ALIAS_SET set.

I think I now understand what happens:
1) the testcase only uses unsigned _BitInt(66), not signed _BitInt(66) aka
_BitInt(66)
2) at some point something calls get_alias_set on unsigned _BitInt(66) [10]
type,
   that does:
  else if (TREE_CODE (t) == ARRAY_TYPE
   && (!TYPE_NONALIASED_COMPONENT (t)
   || TYPE_STRUCTURAL_EQUALITY_P (t)))
set = get_alias_set (TREE_TYPE (t));
   and recurses:
3)
  /* See if the language has special handling for this type.  */
  set = lang_hooks.get_alias_set (t);
  if (set != -1)
return set;
4) c_common_get_alias_set does for the unsigned BITINT_TYPE:
  /* The C standard specifically allows aliasing between signed and
 unsigned variants of the same type.  We treat the signed
 variant as canonical.  */
  if ((TREE_CODE (t) == INTEGER_TYPE || TREE_CODE (t) == BITINT_TYPE)
  && TYPE_UNSIGNED (t))
{
  tree t1 = c_common_signed_type (t);

  /* t1 == t can happen for boolean nodes which are always unsigned.  */
  if (t1 != t)
return get_alias_set (t1);
}
5) that creates new _BitInt(66) BITINT_TYPE, computes its alias set (4), stores
   it in its TYPE_ALIAS_SET, but note, nothing sets TYPE_ALIAS_SET of the t
type
   here (unsigned _BitInt(66)
6) in the caller (see 3), set is not -1, so we return without remembering it
7) in the caller (see 2), we get set = 4; and continue to
  TYPE_ALIAS_SET (t) = set;
   and so now we have the ARRAY_TYPE and _BitInt(66) BITINT_TYPE with
TYPE_ALIAS_SET
   set to 4, but not unsigned _BitInt(66) BITINT_TYPE
8) garbage collection happens, nothing really uses _BitInt(66), it is garbage
collected
9) later on we call get_alias_set on the unsigned _BitInt(66) type (created
just once);
   this does 3)-6) again, creates a new _BitInt(66) type because it doesn't
exist
   anymore, so compute different alias set for it
10) and no TBAA thinks the ARRAY_TYPE (with remembered earlier alias set) can't
alias
   with access through its element type
Now, the reason why we haven't seen this often without _BitInt is I guess that
we stick all the signed and unsigned normal types usually somewhere reachable
from GTY roots, but for BITINT_TYPE that is generally not the case.

So, I think
  /* The C standard specifically allows aliasing between signed and
 unsigned variants of the same type.  We treat the signed
 variant as canonical.  */
  if ((TREE_CODE (t) == INTEGER_TYPE || TREE_CODE (t) == BITINT_TYPE)
  && TYPE_UNSIGNED (t))
{
  tree t1 = c_common_signed_type (t);

  /* t1 == t can happen for boolean nodes which are always unsigned.  */
  if (t1 != t)
return get_alias_set (t1);
}
needs to be updated, so that 1) it remembers the returned get_alias_set in
TYPE_ALIAS_SET (t) 2) that for !TYPE_UNSIGNED (t) for these it calls
c_common_unsigned_type and if t1 != t and TYPE_ALIAS_SET_KNOWN_P (t1) it
returns
that earlier alias set and also sets TYPE_ALIAS_SET.  Plus the above case
should
special case unsigned _BitInt(1) which doesn't have signed counterpart and
guess in that case we want to just return -1 so that the caller creates alias
set.

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-11-22 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

--- Comment #13 from Jakub Jelinek  ---
The type_hash_canon calls for build_nonstandard_integer_type and
build_bitint_type certainly look wrong to me, they are using a different hash
than the hash table will then use:
--- gcc/tree.cc 2025-11-08 08:27:58.115107963 +0100
+++ gcc/tree.cc 2025-11-22 09:38:03.051016099 +0100
@@ -7346,9 +7346,8 @@ build_nonstandard_integer_type (unsigned
   else
 fixup_signed_type (itype);

-  inchash::hash hstate;
-  inchash::add_expr (TYPE_MAX_VALUE (itype), hstate);
-  ret = type_hash_canon (hstate.end (), itype);
+  hashval_t hash = type_hash_canon_hash (itype);
+  ret = type_hash_canon (hash, itype);
   if (precision <= MAX_INT_CACHED_PREC)
 nonstandard_integer_type_cache[precision + unsignedp] = ret;

@@ -7414,9 +7413,8 @@ build_bitint_type (unsigned HOST_WIDE_IN
   else
 fixup_signed_type (itype);

-  inchash::hash hstate;
-  inchash::add_expr (TYPE_MAX_VALUE (itype), hstate);
-  ret = type_hash_canon (hstate.end (), itype);
+  hashval_t hash = type_hash_canon_hash (itype);
+  ret = type_hash_canon (hash, itype);
   if (precision <= MAX_INT_CACHED_PREC)
 (*bitint_type_cache)[precision + unsignedp] = ret;

but why this would cause code generation differences is unclear to me, two
different BITINT_TYPEs with same precision and same signedness should be still
considered uselessly convertible.

[Bug middle-end/122624] AArch64: Miscompile at -O2 with _BitInt

2025-11-21 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122624

Andrew Pinski  changed:

   What|Removed |Added

  Component|tree-optimization   |middle-end

--- Comment #12 from Andrew Pinski  ---
I am thinking we are getting two BITINT_TYPE for 66 precision as 2 different
types which then confuses things.
I can't tell if this is a middle-end issue or a front-end issue.
It might only show up with the gimplification of the the array too.
But it is definitely a GC issue with BITINT.