[Bug rtl-optimization/41033] RTL alias-oracle does not honor -fno-strict-aliasing

2024-07-26 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41033

--- Comment #6 from GCC Commits  ---
The master branch has been updated by Sam James :

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

commit r15-2349-ga75c6295252d0d998a18927dc7510fac965134c4
Author: Sam James 
Date:   Thu Jul 18 08:27:29 2024 +0100

testsuite: Add dg-do run to even more tests

All of these are for wrong-code bugs. Confirmed to be used before but
with no execution.

Tested on x86_64-pc-linux-gnu and checked test logs before/after.

2024-07-26  Sam James  

PR target/7559
PR c++/9704
PR c++/16115
PR c++/19317
PR rtl-optimization/11536
PR target/20322
PR tree-optimization/31966
PR rtl-optimization/41033
PR tree-optimization/67947
* g++.dg/cpp1z/byte1.C: Add dg-do run directive.
* g++.dg/init/call1.C: Ditto.
* g++.dg/init/copy5.C: Ditto.
* g++.dg/opt/nrv9.C: Ditto.
* gcc.dg/20021006-1.c: Ditto.
* gcc.dg/20030721-1.c: Ditto.
* gcc.dg/20050307-1.c: Ditto.
* gcc.dg/pr41033.c: Ditto.
* gcc.dg/torture/pr67947.c: Ditto.
* gcc.dg/tree-ssa/pr31966.c: Ditto.
* gcc.dg/tree-ssa/tailcall-3.c: Ditto.
* gcc.dg/tree-ssa/vrp74.c: Ditto.
* gcc.target/nvptx/abort.c: Fix whitespace in dg directive.

[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5

2024-07-26 Thread xry111 at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039

--- Comment #5 from Xi Ruoyao  ---
*** Bug 116066 has been marked as a duplicate of this bug. ***

[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5

2024-07-25 Thread law at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039

Jeffrey A. Law  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|ASSIGNED|RESOLVED

--- Comment #4 from Jeffrey A. Law  ---
Fixed on the trunk.

[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5

2024-07-25 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039

--- Comment #3 from GCC Commits  ---
The master branch has been updated by Jeff Law :

https://gcc.gnu.org/g:34fb0feca71f763b2fbe832548749666d34a4a76

commit r15-2321-g34fb0feca71f763b2fbe832548749666d34a4a76
Author: Jeff Law 
Date:   Thu Jul 25 12:32:28 2024 -0600

[PR rtl-optimization/116039] Fix life computation for promoted subregs

So this turned out to be a neat little test and while the fuzzer found it
on
RISC-V, I wouldn't be surprised if the underlying issue is also the root
cause
of the loongarch issue with ext-dce.

The key issue is that if we have something like

(set (dest) (any_extend (subreg (source

If the subreg object is marked with SUBREG_PROMOTED and the sign/unsigned
state
matches the any_extend opcode, then combine (and I guess anything using
simplify-rtx) may simplify that to

(set (dest) (source))

That implies that bits outside the mode of the subreg are actually live and
valid.  This needs to be accounted for during liveness computation.

We have to be careful here though. If we're too conservative about setting
additional bits live, then we'll inhibit the desired optimization in the
coremark examples.  To do a good job we need to know the extension opcode.

I'm extremely unhappy with how the use handling works in ext-dce.  It mixes
different conceptual steps and has horribly complex control flow.  It only
handles a subset of the unary/binary opcodes, etc etc.  It's just damn
mess.
It's going to need some more noodling around.

In the mean time this is a bit hacky in that it depends on non-obvious
behavior
to know it can get the extension opcode, but I don't want to leave the
trunk in
a broken state while I figure out the refactoring problem.

Bootstrapped and regression tested on x86 and tested on the crosses. 
Pushing to the trunk.

PR rtl-optimization/116039
gcc/
* ext-dce.cc (ext_dce_process_uses): Add some comments about
concerns
with current code.  Mark additional bit groups as live when we have
an extension of a suitably promoted subreg.

gcc/testsuite
* gcc.dg/torture/pr116039.c: New test.

[Bug c++/115658] char16_t and char32_t aliasing is conserative

2024-07-25 Thread sjames at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658

Sam James  changed:

   What|Removed |Added

 CC||jason at gcc dot gnu.org
 Ever confirmed|0   |1
   Last reconfirmed||2024-07-25
 Status|UNCONFIRMED |NEW

[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5

2024-07-24 Thread law at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039

--- Comment #2 from Jeffrey A. Law  ---
Very interesting little testcase.  This may be the loongarch bug that was
recently reported.

It appears the root cause is this insn (from a hacked up version, so the insn
#s may not match up perfectly):

(insn 82 79 83 4 (set (reg:DI 246 [ _123 ])
(sign_extend:DI (subreg/s/u:QI (reg:DI 260) 0))) "j.c":11:11 128
{*extendqidi2}
 (expr_list:REG_DEAD (reg:DI 260)
(expr_list:REG_EQUAL (sign_extend:DI (mem/c:QI (const:DI (plus:DI
(symbol_ref:DI ("*.LANCHOR0") [flags 0x182])
(const_int 57 [0x39]))) [0 d[9]+0 S1 A8]))


Note the /s/u flags on the SUBREG.  Those tell the optimizers that the value is
actually a sign extended object already and the optimizers can drop the
extraneous extension.   In a subtle way those flags essentially mean that bits
outside the subreg's mode are live.  So we really should have marked additional
groups in (reg 260) as live.  That in turn would have kept key bit groups live
in a different register and inhibited extension removal.

There's a nonzero chance this is also the loongarch bug that just got reported.

[Bug tree-optimization/116034] [12/13 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-24 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

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

https://gcc.gnu.org/g:181f40f5cf8510a16191e4768dadbe2cb7a5c095

commit r14-10507-g181f40f5cf8510a16191e4768dadbe2cb7a5c095
Author: Jakub Jelinek 
Date:   Wed Jul 24 18:00:05 2024 +0200

testsuite: Fix up pr116034.c test for big/pdp endian [PR116061]

Didn't notice the memmove is into an int variable, so the test
was still failing on big endian.

2024-07-24  Jakub Jelinek  

PR tree-optimization/116034
PR testsuite/116061
* gcc.dg/pr116034.c (g): Change type from int to unsigned short.
(foo): Guard memmove call on __SIZEOF_SHORT__ == 2.

(cherry picked from commit 69e69847e21a8d951ab5f09fd3421449564dba31)

[Bug tree-optimization/116034] [12/13 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-24 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

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

https://gcc.gnu.org/g:69e69847e21a8d951ab5f09fd3421449564dba31

commit r15-2274-g69e69847e21a8d951ab5f09fd3421449564dba31
Author: Jakub Jelinek 
Date:   Wed Jul 24 18:00:05 2024 +0200

testsuite: Fix up pr116034.c test for big/pdp endian [PR116061]

Didn't notice the memmove is into an int variable, so the test
was still failing on big endian.

2024-07-24  Jakub Jelinek  

PR tree-optimization/116034
PR testsuite/116061
* gcc.dg/pr116034.c (g): Change type from int to unsigned short.
(foo): Guard memmove call on __SIZEOF_SHORT__ == 2.

[Bug tree-optimization/116034] [12/13 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-23 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

Jakub Jelinek  changed:

   What|Removed |Added

  Known to fail|14.1.1, 15.0|14.1.0
  Known to work||14.1.1, 15.0
Summary|[12/13/14/15 Regression]|[12/13 Regression] wrong
   |wrong code with memcpy()|code with memcpy() from
   |from _Complex unsigned  |_Complex unsigned short at
   |short at|-fno-strict-aliasing -O1
   |-fno-strict-aliasing -O1|and above
   |and above   |

--- Comment #11 from Jakub Jelinek  ---
Fixed for 14.2+/15+ for now.

[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-23 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

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

https://gcc.gnu.org/g:084768c865cd50a6f7ff177db2dbdbb7aadaeee0

commit r14-10501-g084768c865cd50a6f7ff177db2dbdbb7aadaeee0
Author: Jakub Jelinek 
Date:   Tue Jul 23 10:50:29 2024 +0200

ssa: Fix up maybe_rewrite_mem_ref_base complex type handling [PR116034]

The folding into REALPART_EXPR is correct, used only when the mem_offset
is zero, but for IMAGPART_EXPR it didn't check the exact offset value (just
that it is not 0).
The following patch fixes that by using IMAGPART_EXPR only if the offset
is right and using BITFIELD_REF or whatever else otherwise.

2024-07-23  Jakub Jelinek  
Andrew Pinski  

PR tree-optimization/116034
* tree-ssa.cc (maybe_rewrite_mem_ref_base): Only use IMAGPART_EXPR
if MEM_REF offset is equal to element type size.

* gcc.dg/pr116034.c: New test.

(cherry picked from commit b9cefd67a2a464a3c9413e6b3f28e7dc7a9ef162)

[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-23 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

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

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

commit r15-2220-gb9cefd67a2a464a3c9413e6b3f28e7dc7a9ef162
Author: Jakub Jelinek 
Date:   Tue Jul 23 10:50:29 2024 +0200

ssa: Fix up maybe_rewrite_mem_ref_base complex type handling [PR116034]

The folding into REALPART_EXPR is correct, used only when the mem_offset
is zero, but for IMAGPART_EXPR it didn't check the exact offset value (just
that it is not 0).
The following patch fixes that by using IMAGPART_EXPR only if the offset
is right and using BITFIELD_REF or whatever else otherwise.

2024-07-23  Jakub Jelinek  
Andrew Pinski  

PR tree-optimization/116034
* tree-ssa.cc (maybe_rewrite_mem_ref_base): Only use IMAGPART_EXPR
if MEM_REF offset is equal to element type size.

* gcc.dg/pr116034.c: New test.

[Bug rtl-optimization/116039] [15 Regression] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5

2024-07-22 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039

Andrew Pinski  changed:

   What|Removed |Added

   Last reconfirmed||2024-07-23
 Ever confirmed|0   |1
 Status|UNCONFIRMED |ASSIGNED

--- Comment #1 from Andrew Pinski  ---
.

[Bug rtl-optimization/116039] [15] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5

2024-07-22 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039

Andrew Pinski  changed:

   What|Removed |Added

  Component|target  |rtl-optimization
 CC||pinskia at gcc dot gnu.org
   Target Milestone|--- |15.0

[Bug target/116039] New: [15] rv64gc miscompile at -O3 with -fno-strict-aliasing since r15-1901-g98914f9eba5

2024-07-22 Thread patrick at rivosinc dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116039

Bug ID: 116039
   Summary: [15] rv64gc miscompile at -O3 with
-fno-strict-aliasing since r15-1901-g98914f9eba5
   Product: gcc
   Version: 15.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: target
  Assignee: unassigned at gcc dot gnu.org
  Reporter: patrick at rivosinc dot com
  Target Milestone: ---

Testcase:
int c[12];
char d[12];
int *f = c;
int *z = (int *)1;
long long y;
int main() {
  c[9] = 0xff;
  for (int i = 0; i < 12; i += 3)
d[9] = z ? f[i] : 0;
  for (long i = 0; i < 12; ++i)
y ^= d[i];
  __builtin_printf("%llu\n", y);
}

Commands:
> /scratch/tc-testing/tc-compiler-fuzz-trunk/build-gcv/bin/riscv64-unknown-linux-gnu-gcc
>  -O3 red.c -o user-config.out -fsigned-char -fno-strict-aliasing -fwrapv
> QEMU_CPU=rv64,vlen=128,rvv_ta_all_1s=true,rvv_ma_all_1s=true,v=true,vext_spec=v1.0,zve32f=true,zve64f=true
>  timeout --verbose -k 0.1 4 
> /scratch/tc-testing/tc-compiler-fuzz-trunk/build-gcv/bin/qemu-riscv64 
> user-config.out
255
> /scratch/tc-testing/tc-compiler-fuzz-trunk/build-gcv/bin/riscv64-unknown-linux-gnu-gcc
>  -O2 red.c -o user-config.out -fsigned-char -fno-strict-aliasing -fwrapv
> QEMU_CPU=rv64,vlen=128,rvv_ta_all_1s=true,rvv_ma_all_1s=true,v=true,vext_spec=v1.0,zve32f=true,zve64f=true
>  timeout --verbose -k 0.1 4 
> /scratch/tc-testing/tc-compiler-fuzz-trunk/build-gcv/bin/qemu-riscv64 
> user-config.out
18446744073709551615

First bad commit: r15-1901-g98914f9eba5

Found via fuzzer.

[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

--- Comment #8 from Andrew Pinski  ---
(In reply to Jakub Jelinek from comment #7)
> Created attachment 58723 [details]
> gcc15-pr116034.patch
> 
> Full untested patch.

This looks good to me. This is basically the same as the patch which I was
coming up with too.

[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

Jakub Jelinek  changed:

   What|Removed |Added

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

--- Comment #7 from Jakub Jelinek  ---
Created attachment 58723
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58723=edit
gcc15-pr116034.patch

Full untested patch.

[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread zsojka at seznam dot cz via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

--- Comment #6 from Zdenek Sojka  ---
(In reply to Sam James from comment #1)
> on godbolt, 6.5 looks OK, and 7.1 starts to fail for amd64.

My apologies; the results for gcc-6.5.0 and gcc-7.5.0 were not correct due to
linking errors. Indeed 6.5.0 works, and 7.5.0 fails.

[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

Jakub Jelinek  changed:

   What|Removed |Added

 CC||jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek  ---
Started with r7-2818-gebfa15ab654738fcc926a506b3788a303958fa02

[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

--- Comment #4 from Andrew Pinski  ---
I think the issue is in maybe_rewrite_mem_ref_base :
  else if (TREE_CODE (TREE_TYPE (sym)) == COMPLEX_TYPE
   && useless_type_conversion_p (TREE_TYPE (*tp),
 TREE_TYPE (TREE_TYPE (sym
{
  *tp = build1 (integer_zerop (TREE_OPERAND (*tp, 1))
? REALPART_EXPR : IMAGPART_EXPR,
TREE_TYPE (*tp), sym);
}


It only checks for 0 and non-zero rather than 0 and size (of type) like it is
done in gimple-fold.cc and fold-const.cc.

Which means it has been latent since r0-107202-g64a3d6470e5eb7 and exposed by
something started to be optimized in GCC 7.

[Bug tree-optimization/116034] [12/13/14/15 Regression] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

Andrew Pinski  changed:

   What|Removed |Added

 Ever confirmed|0   |1
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2024-07-22
   Target Milestone|--- |12.5
Summary|wrong code with memcpy()|[12/13/14/15 Regression]
   |from _Complex unsigned  |wrong code with memcpy()
   |short at|from _Complex unsigned
   |-fno-strict-aliasing -O1|short at
   |and above   |-fno-strict-aliasing -O1
   ||and above

--- Comment #3 from Andrew Pinski  ---
Confirmed.

Looks like the code that folds MEM[(charD.10 * {ref-all}) + 1B] into
REAL/IMAG is going wrong and should not have folded it into that. I have not
looked into why though.

[Bug tree-optimization/116034] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

--- Comment #2 from Andrew Pinski  ---
Folding statement: _1 =  + 1;
Queued stmt for removal.  Folds to:  <__complex__ short unsigned int>
[(void *) + 1B]
Folding statement: _3 = MEM  [(char * {ref-all})_1];
Folded into: _3 = MEM  [(char * {ref-all}) + 1B];

That is ok, but then we change it into:
  _3 = IMAGPART_EXPR ;

[Bug tree-optimization/116034] wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread sjames at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

Sam James  changed:

   What|Removed |Added

 CC||sjames at gcc dot gnu.org

--- Comment #1 from Sam James  ---
on godbolt, 6.5 looks OK, and 7.1 starts to fail for amd64.

[Bug tree-optimization/116034] New: wrong code with memcpy() from _Complex unsigned short at -fno-strict-aliasing -O1 and above

2024-07-22 Thread zsojka at seznam dot cz via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116034

Bug ID: 116034
   Summary: wrong code with memcpy() from _Complex unsigned short
at -fno-strict-aliasing -O1 and above
   Product: gcc
   Version: 15.0
Status: UNCONFIRMED
  Keywords: wrong-code
  Severity: normal
  Priority: P3
 Component: tree-optimization
  Assignee: unassigned at gcc dot gnu.org
  Reporter: zsojka at seznam dot cz
  Target Milestone: ---
  Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu

Created attachment 58721
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58721=edit
reduced testcase

Output:
$ x86_64-pc-linux-gnu-gcc -O1 -fno-strict-aliasing testcase.c
$ ./a.out
Aborted

$ x86_64-pc-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r15-2206-20240722194717-g6f81b7fa799-checking-yes-rtl-df-extra-nobootstrap-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/15.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--disable-bootstrap --with-cloog --with-ppl --with-isl
--build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu
--target=x86_64-pc-linux-gnu --with-ld=/usr/bin/x86_64-pc-linux-gnu-ld
--with-as=/usr/bin/x86_64-pc-linux-gnu-as --enable-libsanitizer
--disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r15-2206-20240722194717-g6f81b7fa799-checking-yes-rtl-df-extra-nobootstrap-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 15.0.0 20240722 (experimental) (GCC) 

This is also failing on aarch64-unknown-linux-gnu and
powerpc64le-unknown-linux-gnu, but it's OK on mips64el-unknown-linux-gnuabi64
and riscv64-unknown-linux-gnu.

[Bug tree-optimization/113630] [12 Regression] -fno-strict-aliasing introduces out-of-bounds memory access

2024-07-19 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630

Richard Biener  changed:

   What|Removed |Added

   Target Milestone|11.5|12.5

--- Comment #10 from Richard Biener  ---
GCC 11 branch is being closed.

[Bug lto/112732] during IPA pass: *free_lang_data ICE: 'verify_type' failed: type variant with 'TYPE_ALIAS_SET_KNOWN_P' with -Os -Wstrict-aliasing=2 -flto

2024-07-08 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112732

--- Comment #7 from GCC Commits  ---
The releases/gcc-11 branch has been updated by Richard Biener
:

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

commit r11-11561-ge7879391bb2b86606d0ce35ed97eccc108970e36
Author: Richard Biener 
Date:   Tue Nov 28 12:36:21 2023 +0100

middle-end/112732 - stray TYPE_ALIAS_SET in type variant

The following fixes a stray TYPE_ALIAS_SET in a type variant built
by build_opaque_vector_type which is diagnosed by type checking
enabled with -flto.

PR middle-end/112732
* tree.c (build_opaque_vector_type): Reset TYPE_ALIAS_SET
of the newly built type.

(cherry picked from commit f26d68d5d128c86faaceeb81b1e8f22254ad53df)

[Bug c++/115658] char16_t and char32_t aliasing is conserative

2024-06-28 Thread tom at honermann dot net via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658

--- Comment #3 from Tom Honermann  ---
In retrospect, I think I misunderstood Andrew's motivation for filing this
issue.

There is a difference of behavior between gcc and clang with regard to aliasing
of `char16_t` and `char32_t` with respect to other types. This is illustrated
by the following example as demonstrated at
https://www.godbolt.org/z/PsMsPMa73.

Please note that (at least) `-O2` is necessary to reliably demonstrate
differences in behavior. Additionally, the use of `-fshort-wchar` to influence
the size of `wchar_t` affects behavior.

```
template
U f(T *p, U *q) {
  *p = 1;
  U u = *q;
  *p = 2;
  return u;
}
template wchar_t f(char16_t*, wchar_t*);
template unsigned short f(char16_t*, unsigned short*);
template wchar_t f(char32_t*, wchar_t*);
template unsigned int f(char32_t*, unsigned int*);
```

The test case exercises dead store elimination in the presence of aliasing
types. If `T` may alias `U`, then the write of `1` to `*p` is observable by
`*q`, but may otherwise be eliminated due to the later write of `2` to `*p`.

For Clang, there is no aliasing between any of these types and the store of `1`
to `*p` is always eliminated.

For MSVC, it appears that either dead store elimination is not performed at
all, or aliasing is permitted across all of these types (even when the size
differs).

For gcc with `-fshort-wchar`, there appear to be two alias sets:
- `wchar_t`, `char16_t`, and `unsigned short`.
- `char32_t` and `unsigned int`.

For gcc without `-fshort-wchar`, there are also two alias sets, but they are
not symmetric in the presence of that option. Note that `char32_t` never
aliases with `wchar_t` even when they have the same size. This asymmetry is
explainable in consideration of compatibility with MSVC (where `wchar_t` is
always 16-bit).
- `char16_t` and `unsigned short`.
- `char32_t` and `unsigned int`.

Adding the following explicit template instantiations demonstrates that all of
gcc, clang, and MSVC permit aliasing between the set of `char`, `unsigned
char`, and `char8_t` (because `char` and `unsigned char` are permitted to alias
all types). https://www.godbolt.org/z/Pjxb661Y7.

```
template char f(char8_t*, char*);
template unsigned char f(char8_t*, unsigned char*);
```

To reiterate, I think the current gcc behavior is correct and defensible given
two goals:
- A desire to match MSVC behavior in the limited context of a 16-bit `wchar_t`
type.
- A desire to match C behavior with respect to `char16_t` and `char32_t`
aliasing the underlying types of `uint_least16_t` and `uint_least32_t` (the
former are typedefs in C).

[Bug c++/115658] char16_t and char32_t aliasing is conserative

2024-06-26 Thread tom at honermann dot net via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658

Tom Honermann  changed:

   What|Removed |Added

 CC||tom at honermann dot net

--- Comment #2 from Tom Honermann  ---
I think the prior comments regarding the aliasing sets refer to portions of
c_common_get_alias_set() in gcc/gcc/c-family/c-common.cc, specifically the
lines at
https://github.com/gcc-mirror/gcc/blob/629257bcb81434117f1e9c68479032563176dc0c/gcc/c-family/c-common.cc#L3892-L3895.

I think the alias sets are right; they are consistent with what the C++
standard states in [basic.lval]p11 (http://eel.is/c++draft/basic.lval#11).

> So maybe we really should keep on treating them the same.
and maybe change char8_t back to similar as unsigned char ...

I don't think that should be necessary. P0482R6 was explicit in its intent that
char8_t not be an aliasing type (http://wg21.link/p0482r6#proposal).

[Bug c++/115658] char16_t and char32_t aliasing is conserative

2024-06-25 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658

--- Comment #1 from Andrew Pinski  ---
Though I should note
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2626r0.pdf and
https://github.com/sg16-unicode/sg16-meetings/tree/master#may-22nd-2024


So maybe we really should keep on treating them the same.
and maybe change char8_t back to similar as unsigned char ...

[Bug c++/115658] New: char16_t and char32_t aliasing is conserative

2024-06-25 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658

Bug ID: 115658
   Summary: char16_t and char32_t aliasing is conserative
   Product: gcc
   Version: 15.0
Status: UNCONFIRMED
  Keywords: alias, missed-optimization
  Severity: enhancement
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: pinskia at gcc dot gnu.org
  Target Milestone: ---

when char8_t was added, a new aliasing set was done:
r9-5405-g2d91f79dc990f8

But when char16_t and char32_t was added (for GCC 4.4/C++11):
r0-88474-gc466b2cd136139

That was not done.

Maybe it should be done now.

Noticed from https://github.com/sg16-unicode/sg16/issues/67 .

[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double

2024-06-21 Thread davidhu0903ex3 at gmail dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115552

--- Comment #3 from davidhcefx  ---
Thanks for your explanation, we now understand the problem.

[Bug tree-optimization/107467] [12/13/14/15 Regression] Miscompilation involing -Os , -flto and -fno-strict-aliasing since r12-656-ga564da506f52be66

2024-06-20 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107467

Richard Biener  changed:

   What|Removed |Added

   Target Milestone|12.4|12.5

--- Comment #11 from Richard Biener  ---
GCC 12.4 is being released, retargeting bugs to GCC 12.5.

[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double

2024-06-19 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115552

--- Comment #2 from Andrew Pinski  ---
So basically since main writes via a double type but then spookyhash_short
reads via a uint64_t type there is an alias violation.

Using an union to change the pointer type does not change there is an alias
violation happening. You need to use memcpy for copying from one type to
another in this case. 

Instead of doing:
c += u.p64[0];

You could do load_64([0]) and have load_64 defined to be:
```
static inline uint64_t load_64(void *a)
{
  uint64_t t;
  memcpy(, a, sizeof(t));
  return t;
}
```

[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double

2024-06-19 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115552

Andrew Pinski  changed:

   What|Removed |Added

 Resolution|--- |INVALID
 Status|UNCONFIRMED |RESOLVED

--- Comment #1 from Andrew Pinski  ---

union {
const uint8_t *p8;
uint32_t *p32;
uint64_t *p64;
size_t i;
} u;


Does not get around aliasing rules of C/C++.

[Bug c/115552] New: wrong code at -O2 and above when strict-aliasing with uint128 and double

2024-06-19 Thread davidhu0903ex3 at gmail dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115552

Bug ID: 115552
   Summary: wrong code at -O2 and above when strict-aliasing with
uint128 and double
   Product: gcc
   Version: 12.2.0
Status: UNCONFIRMED
  Keywords: alias, wrong-code
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: davidhu0903ex3 at gmail dot com
  Target Milestone: ---

Created attachment 58469
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58469=edit
preproccessed file

=== Summary ===

When turning on -O2 optimization or above, the produced hash output is
different when without -O2, unless adding `-fno-strict-aliasing`. To my
investigation, the issue arise only when either:
  1. output to a uint128_t*
  2. input with a double*

=== Detail Explanation ===

The program produced the following output when compiled with -O1

  60d8f0889f4b7d88
  3bcf47a7eecabc5d
  1b153029d058b5115490463dd8be0468
  1b153029d058b5115490463dd8be0468

However, it produced different output with -O2

  e09546989572a77c
  95fe807a3fab3ad5
  b8b1ef1e717bc0d4818be1e9343867f6
  

The first two lines are the case where the inputs are double* pointers. The
third line is the case where the output are written to a uint128_t* pointer.
The last line is the case of passing a uint128_t by value to `foo` before dump.

The warning didn't complain about strict-aliasing, but rather an ambiguous
message:

  'd1' may be used uninitialized [-Wmaybe-uninitialized]

However, those variables are already initialized in `main`:

  double d1 = 1;
  double d2 = 2;

>From the generated assembly, the initialization of the inputs seems missing, so
perhaps that's the reason of the warning message. However, in production our
code didn't emit "-Wmaybe-uninitialized" at all, so that's what surprises us.

With Compiler Explorer (godbolt.org), I can roughly locate that this behavior
appear after version 10.5.0 and until 14.1.0.

=== Environment ===

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/12/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 12.2.0-14'
--with-bugurl=file:///usr/share/doc/gcc-12/README.Bugs
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-12
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug
--enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new
--enable-gnu-unique-object --disable-vtable-verify --enable-plugin
--enable-default-pie --with-system-zlib --enable-libphobos-checking=release
--with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch
--disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64
--with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none=/build/gcc-12-bTRWOB/gcc-12-12.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-12-bTRWOB/gcc-12-12.2.0/debian/tmp-gcn/usr
--enable-offload-defaulted --without-cuda-driver --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.0 (Debian 12.2.0-14) 
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc'
'-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/12/cc1 -E -quiet -v -imultiarch x86_64-linux-gnu
poc.c -mtune=generic -march=x86-64 -Wall -Wextra -O2 -fpch-preprocess
-fasynchronous-unwind-tables -o poc.i
ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu"
ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/12/include-fixed"
ignoring nonexistent directory
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/x86_64-linux-gnu/12/include
 /usr/local/include
 /usr/include/x86_64-linux-gnu
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc'
'-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/12/cc1 -fpreprocessed poc.i -quiet -dumpbase
poc.c -dumpbase-ext .c -mtune=generic -march=x86-64 -O2 -Wall -Wextra -version
-fasynchronous-unwind-tables -o poc.s
GNU C17 (Debian 12.2.0-14) version 12.2.0 (x86_64-linux-gnu)
compiled by GNU C version 12.2.0, GMP version 6.2.1, MPFR version
4.1.1-p1, MPC version 1.3.1, isl version isl-0.25-GMP

warning: MPFR header version 4.1.1-p1 differs from library version 4.2.0.
GGC heuristics: --pa

Re: [PATCH v2 0/4] Libatomic: Cleanup ifunc selector and aliasing

2024-06-12 Thread Richard Sandiford
Victor Do Nascimento  writes:
> Changes in V2:
>
> As explained in patch v2 1/4, it has become clear that the current
> approach of querying assembler support for newer architectural
> extensions at compile time is undesirable both from a maintainability
> as well as a consistency standpoint - Different compiled versions of
> Libatomic may have different features depending on the machine on
> which they were built.
>
> These issues make for difficult testing as the explosion in number of
> `#ifdef' guards makes maintenance error-prone and the dependence on
> binutils version means that, as well as deploying changes for testing
> in a variety of target configurations, testing must also involve
> compiling the library on an increasing number of host configurations,
> meaning that the chance of bugs going undetected increases (as was
> proved in the pre-commit CI which, due to the use of an older version
> of Binutils, picked up on a runtime-error that had hitherto gone
> unnoticed).
>
> We therefore do away with the use of all assembly instructions
> dependent on Binutils 2.42, choosing to replace them with `.inst's
> instead.  This eliminates the latent bug picked up by CI and will
> ensure consistent builds of Libatomic across all versions of Binutils.

Nice!  Thanks for doing this.  It seems much cleaner and more flexible
than the current approach.

Thanks also for the clear organisation of the series.

OK for trunk.  (For the record, I didn't hand-check the encodings of the
.insts ...)

Richard

> ---
>
> The recent introduction of the optional LSE128 and RCPC3 architectural
> extensions to AArch64 has further led to the increased flexibility of
> atomic support in the architecture, with many extensions providing
> support for distinct atomic operations, each with different potential
> applications in mind.
>
> This has led to maintenance difficulties in Libatomic, in particular
> regarding the way the ifunc selector is generated via a series of
> macro expansions at compile-time.
>
> Until now, irrespective of the atomic operation in question, all atomic
> functions for a particular operand size were expected to have the same
> number of ifunc alternatives, meaning that a one-size-fits-all
> approach could reasonably be taken for the selector.
>
> This meant that if, hypothetically, for a particular architecture and
> operand size one particular atomic operation was to have 3 different
> implementations associated with different extensions, libatomic would
> likewise be required to present three ifunc alternatives for all other
> atomic functions.
>
> The consequence in the design choice was the unnecessary use of
> function aliasing and the unwieldy code which resulted from this.
>
> This patch series attempts to remediate this issue by making the
> preprocessor macros defining the number of ifunc alternatives and
> their respective selection functions dependent on the file importing
> the ifunc selector-generating framework.
>
> all files are given `LAT_' macros, defined at the beginning
> and undef'd at the end of the file.  It is these macros that are
> subsequently used to fine-tune the behaviors of `libatomic_i.h' and
> `host-config.h'.
>
> In particular, the definition of the `IFUNC_NCOND(N)' and
> `IFUNC_COND_' macros in host-config.h can now be guarded behind
> these new file-specific macros, which ultimately control what the
> `GEN_SELECTOR(X)' macro in `libatomic_i.h' expands to.  As both of
> these headers are imported once per file implementing some atomic
> operation, fine-tuned control is now possible.
>
> Regtested with both `--enable-gnu-indirect-function' and
> `--disable-gnu-indirect-function' configurations on armv9.4-a target
> with LRCPC3 and LSE128 support and without.
>
> Victor Do Nascimento (4):
>   Libatomic: AArch64: Convert all lse128 assembly to .insn directives
>   Libatomic: Define per-file identifier macros
>   Libatomic: Make ifunc selector behavior contingent on importing file
>   Libatomic: Clean up AArch64 `atomic_16.S' implementation file
>
>  libatomic/acinclude.m4   |  18 -
>  libatomic/auto-config.h.in   |   3 -
>  libatomic/cas_n.c|   2 +
>  libatomic/config/linux/aarch64/atomic_16.S   | 511 +--
>  libatomic/config/linux/aarch64/host-config.h |  35 +-
>  libatomic/configure  |  43 --
>  libatomic/configure.ac   |   3 -
>  libatomic/exch_n.c   |   2 +
>  libatomic/fadd_n.c   |   2 +
>  libatomic/fand_n.c   |   2 +
>  libatomic/fence.c|   2 +
>  libatomic/fenv.c  

[Bug c/115096] -fstrict-aliasing miscompilation or missing warning

2024-06-11 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115096

Andrew Pinski  changed:

   What|Removed |Added

 CC||Explorer09 at gmail dot com

--- Comment #3 from Andrew Pinski  ---
*** Bug 115437 has been marked as a duplicate of this bug. ***

[Bug lto/80379] Redundant note: code may be misoptimized unless -fno-strict-aliasing is used

2024-06-11 Thread andi-gcc at firstfloor dot org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80379

--- Comment #5 from Andi Kleen  ---
This bug is about printing a unnecessary message. If your code is actually
miscompiled even with -fno-strict-aliasing set (so it is ignored somewhere) it
is something different and you would need a test case to debug.

[PATCH v2 0/4] Libatomic: Cleanup ifunc selector and aliasing

2024-06-11 Thread Victor Do Nascimento
Changes in V2:

As explained in patch v2 1/4, it has become clear that the current
approach of querying assembler support for newer architectural
extensions at compile time is undesirable both from a maintainability
as well as a consistency standpoint - Different compiled versions of
Libatomic may have different features depending on the machine on
which they were built.

These issues make for difficult testing as the explosion in number of
`#ifdef' guards makes maintenance error-prone and the dependence on
binutils version means that, as well as deploying changes for testing
in a variety of target configurations, testing must also involve
compiling the library on an increasing number of host configurations,
meaning that the chance of bugs going undetected increases (as was
proved in the pre-commit CI which, due to the use of an older version
of Binutils, picked up on a runtime-error that had hitherto gone
unnoticed).

We therefore do away with the use of all assembly instructions
dependent on Binutils 2.42, choosing to replace them with `.inst's
instead.  This eliminates the latent bug picked up by CI and will
ensure consistent builds of Libatomic across all versions of Binutils.

---

The recent introduction of the optional LSE128 and RCPC3 architectural
extensions to AArch64 has further led to the increased flexibility of
atomic support in the architecture, with many extensions providing
support for distinct atomic operations, each with different potential
applications in mind.

This has led to maintenance difficulties in Libatomic, in particular
regarding the way the ifunc selector is generated via a series of
macro expansions at compile-time.

Until now, irrespective of the atomic operation in question, all atomic
functions for a particular operand size were expected to have the same
number of ifunc alternatives, meaning that a one-size-fits-all
approach could reasonably be taken for the selector.

This meant that if, hypothetically, for a particular architecture and
operand size one particular atomic operation was to have 3 different
implementations associated with different extensions, libatomic would
likewise be required to present three ifunc alternatives for all other
atomic functions.

The consequence in the design choice was the unnecessary use of
function aliasing and the unwieldy code which resulted from this.

This patch series attempts to remediate this issue by making the
preprocessor macros defining the number of ifunc alternatives and
their respective selection functions dependent on the file importing
the ifunc selector-generating framework.

all files are given `LAT_' macros, defined at the beginning
and undef'd at the end of the file.  It is these macros that are
subsequently used to fine-tune the behaviors of `libatomic_i.h' and
`host-config.h'.

In particular, the definition of the `IFUNC_NCOND(N)' and
`IFUNC_COND_' macros in host-config.h can now be guarded behind
these new file-specific macros, which ultimately control what the
`GEN_SELECTOR(X)' macro in `libatomic_i.h' expands to.  As both of
these headers are imported once per file implementing some atomic
operation, fine-tuned control is now possible.

Regtested with both `--enable-gnu-indirect-function' and
`--disable-gnu-indirect-function' configurations on armv9.4-a target
with LRCPC3 and LSE128 support and without.

Victor Do Nascimento (4):
  Libatomic: AArch64: Convert all lse128 assembly to .insn directives
  Libatomic: Define per-file identifier macros
  Libatomic: Make ifunc selector behavior contingent on importing file
  Libatomic: Clean up AArch64 `atomic_16.S' implementation file

 libatomic/acinclude.m4   |  18 -
 libatomic/auto-config.h.in   |   3 -
 libatomic/cas_n.c|   2 +
 libatomic/config/linux/aarch64/atomic_16.S   | 511 +--
 libatomic/config/linux/aarch64/host-config.h |  35 +-
 libatomic/configure  |  43 --
 libatomic/configure.ac   |   3 -
 libatomic/exch_n.c   |   2 +
 libatomic/fadd_n.c   |   2 +
 libatomic/fand_n.c   |   2 +
 libatomic/fence.c|   2 +
 libatomic/fenv.c |   2 +
 libatomic/fior_n.c   |   2 +
 libatomic/flag.c |   2 +
 libatomic/fnand_n.c  |   2 +
 libatomic/fop_n.c|   2 +
 libatomic/fsub_n.c   |   2 +
 libatomic/fxor_n.c   |   2 +
 libatomic/gcas.c |   2 +
 libatomic/gexch.c|   2 +
 libatomic/glfree.c   |   2 +
 libatomic/gload.c|   2 +
 libatomic/gstore.c   |   2 +
 libatomic/load_n.c   |   2 +
 libatomic/store_n.c

[Bug lto/115432] Building a program with -flto generates wrong code (missing the call to a function) unless -fno-strict-aliasing

2024-06-11 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115432

Richard Biener  changed:

   What|Removed |Added

 Resolution|--- |INVALID
 Status|UNCONFIRMED |RESOLVED

--- Comment #2 from Richard Biener  ---
struct file_output_stream
{

union
{
void *voidp;
int fd;
} data;

const output_stream_vtbl* vtbl;
};

struct output_stream
{
void* data;
const output_stream_vtbl* vtbl;
};

those are two unrelated types.  Doing

 ((file_output_stream *)p)->vtbl = x;
 ... = ((output_stream *)p)->vtbl;

is invoking undefined behavior (unless -fno-strict-aliasing).

[Bug lto/115432] Building a program with -flto generates wrong code (missing the call to a function) unless -fno-strict-aliasing

2024-06-11 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115432

--- Comment #1 from Richard Biener  ---
In case output_stream is not the same or derived from file_output_stream
or contains a file_output_stream object as first member you invoke undefined
behavior when the calls following might read from the object via output_stream
or another alltogether different type (buffer_output_stream?).

[Bug lto/115432] New: Building a program with -flto generates wrong code (missing the call to a function) unless -fno-strict-aliasing

2024-06-11 Thread Eric.Diaz.Fernandez at eurid dot eu via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115432

Bug ID: 115432
   Summary: Building a program with -flto generates wrong code
(missing the call to a function) unless
-fno-strict-aliasing
   Product: gcc
   Version: 14.1.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: lto
  Assignee: unassigned at gcc dot gnu.org
  Reporter: Eric.Diaz.Fernandez at eurid dot eu
  Target Milestone: ---

Created attachment 58403
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58403=edit
All the .i files and the output of the build.

Summary:

-flto generated code is wrong: resulting binary doesn't contain the call to an
initialisation function (making the program crash, obviously)
Using -fno-strict-aliasing disables the bug

Tried both on:

gcc (Ubuntu 13.2.0-23ubuntu4) 13.2.0 / Ubuntu 24.04
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-linux-gnu/13/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
13.2.0-23ubuntu4' --with-bugurl=file:///usr/share/doc/gcc-13/README.Bugs
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-13
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/libexec --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug
--enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new
--enable-libstdcxx-backtrace --enable-gnu-unique-object --disable-vtable-verify
--enable-plugin --enable-default-pie --with-system-zlib
--enable-libphobos-checking=release --with-target-system-zlib=auto
--enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
--enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none=/build/gcc-13-uJ7kn6/gcc-13-13.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-13-uJ7kn6/gcc-13-13.2.0/debian/tmp-gcn/usr
--enable-offload-defaulted --without-cuda-driver --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.2.0 (Ubuntu 13.2.0-23ubuntu4) 


gcc (GCC) 14.1.1 20240522 / Arch
gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/14.1.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure
--enable-languages=ada,c,c++,d,fortran,go,lto,m2,objc,obj-c++,rust
--enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib
--mandir=/usr/share/man --infodir=/usr/share/info
--with-bugurl=https://gitea.artixlinux.org/packages/gcc/issues
--with-build-config=bootstrap-lto --with-linker-hash-style=gnu
--with-system-zlib --enable-__cxa_atexit --enable-cet=auto
--enable-checking=release --enable-clocale=gnu --enable-default-pie
--enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object
--enable-libstdcxx-backtrace --enable-link-serialization=1
--enable-linker-build-id --enable-lto --enable-multilib --enable-plugin
--enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch
--disable-werror
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 14.1.1 20240522 (GCC) 


The bug is an initialisation function missing from the generated assembly.
The bug only occurrs when doing the build of the full program using LTO and
only for one of the program (yadifad).
The only difference I can think of between that program and the other ones
(when initialisation is concerned) is that yadifad calls the dnscore_init_ex
function in two locations and in two different manners:
_ It can call it using a constant bitmask indicating only a minimal amount of
subsystems are to be initialised.
_ It can call it using a varibable bitmask indicating either a minimal amount
or all of the subsystems are to be initialised.

I've tried to make a sample case but it appears any modification makes the
issue disappear. So all I have is the full build.
I've read that you only want the .i files and I'm attaching the ones directly
used here. I can't attach them all as the compressed archive is bigger than
2MB.
The archive also contains the output of the build.

One of the files compilation line:

gcc -DHAVE_CONFIG_H -I. -I/home/guest/src/yadifa-2.6/sbin/yadifad -I../..   
-D_THREAD_SAFE -D_REENTRANT -D_FILE_OFFSET_BITS=64 -I/tmp/ltonofat/sbin/yadifad
-I/tmp/ltonofat/sbin/yadifad/include
-I/home/guest/src/yadifa-2.6/sbin/yadifad/include  -fno-ident -ansi -pedantic
-Wall -Wno-unknown-pragmas -Werror=missing-field-initializers -std=gnu11
-mtune=native  -DUSES_GCC  -DPREFIX='"/u

[Bug lto/80379] Redundant note: code may be misoptimized unless -fno-strict-aliasing is used

2024-06-11 Thread Eric.Diaz.Fernandez at eurid dot eu via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80379

Eric Diaz Fernandez  changed:

   What|Removed |Added

 CC||Eric.Diaz.Fernandez at eurid 
dot e
   ||u

--- Comment #4 from Eric Diaz Fernandez  
---
I believe I've hit that bug. I was about to post a report it when I found this
one.

When building one of my programs with -flto but without -fno-strict-aliasing
the resulting assembly misses the call to an initialization function.
The current workaround suggested to the maintainer is to not use -flto

Is there anything I can add to help fixing the issue?

[Bug middle-end/115405] wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above

2024-06-10 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115405

--- Comment #3 from Richard Biener  ---
It's not visible but I assume that _4 doesn't have _BitInt(17) type?

The

if (known_eq (offset, 0)
&& !reverse
&& poly_int_tree_p (TYPE_SIZE (type), _size)
&& known_eq (GET_MODE_BITSIZE (DECL_MODE (base)), type_size))

check tries to assess that no extension is required, does it work if you
adjust that for the _BitInt case?

OTOH the reduce_bit_field handling in VIEW_CONVERT_EXPR expansion looks
misplaced - shouldn't it be before the INTEGRAL_TYPE_P handling?

[Bug middle-end/115405] wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above

2024-06-10 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115405

Jakub Jelinek  changed:

   What|Removed |Added

 CC||rguenth at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek  ---
The IL before expansion doesn't have a VCE, just
MEM[(_BitInt(17) *)]
where b has unsigned _BitInt(17) type.
Then
/* Handle expansion of non-aliased memory with non-BLKmode.  That
   might end up in a register.  */
if (mem_ref_refers_to_non_mem_p (exp))
  {
poly_int64 offset = mem_ref_offset (exp).force_shwi ();
base = TREE_OPERAND (base, 0);
poly_uint64 type_size;
if (known_eq (offset, 0)
&& !reverse
&& poly_int_tree_p (TYPE_SIZE (type), _size)
&& known_eq (GET_MODE_BITSIZE (DECL_MODE (base)), type_size))
  return expand_expr (build1 (VIEW_CONVERT_EXPR, type, base),
  target, tmode, modifier);
in expr.cc turns that into a VCE.
And VCE doesn't really EXTEND_BITINT because the upper bits generally in a VCE
are undefined.
Finally the NOP_EXPR expansion sees a SImode signed _BitInt(17) extension to
SImode 32-bit int and assumes for something in an automatic var to be already
properly extended.

Dunno where the bug is, whether trying to use VCE for the _BitInt cases as in
the snippet above, or VCE not doing EXTEND_BITINT, or NOP_EXPR handling that
should extend even in this case.

[Bug middle-end/115405] wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above

2024-06-09 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115405

Andrew Pinski  changed:

   What|Removed |Added

   Last reconfirmed||2024-06-09
 Status|UNCONFIRMED |NEW
 Ever confirmed|0   |1
  Component|rtl-optimization|middle-end

--- Comment #1 from Andrew Pinski  ---
Confirmed. Looks to be a mising sign extend when expanding the VCE like
MEM_REF.


;; _4 = MEM[(_BitInt(17) *)];

(insn 8 7 9 (parallel [
(set (reg:SI 103)
(and:SI (reg/v:SI 100 [ bD.2205 ])
(const_int 131071 [0x1])))
(clobber (reg:CC 17 flags))
]) "/app/example.cpp":6:10 -1
 (nil))

(insn 9 8 0 (set (reg:SI 99 [ _4 ])
(reg:SI 103)) "/app/example.cpp":6:10 -1
 (nil))

;; b ={v} {CLOBBER(eos)};

(nil)

;; if (_4 != -65536)

(insn 10 9 11 (set (reg:CCZ 17 flags)
(compare:CCZ (reg:SI 99 [ _4 ])
(const_int -65536 [0x]))) "/app/example.cpp":13:6
-1
 (nil))

[Bug tree-optimization/115405] New: wrong code with _BitInt() sign-extension with -fno-strict-aliasing -O1 and above

2024-06-09 Thread zsojka at seznam dot cz via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115405

Bug ID: 115405
   Summary: wrong code with _BitInt() sign-extension with
-fno-strict-aliasing -O1 and above
   Product: gcc
   Version: 15.0
Status: UNCONFIRMED
  Keywords: wrong-code
  Severity: normal
  Priority: P3
 Component: tree-optimization
  Assignee: unassigned at gcc dot gnu.org
  Reporter: zsojka at seznam dot cz
CC: jakub at gcc dot gnu.org
  Target Milestone: ---
  Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu

Created attachment 58389
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58389=edit
reduced testcase

Output:
$ x86_64-pc-linux-gnu-gcc -O0 testcase.c
$ ./a.out
$ x86_64-pc-linux-gnu-gcc -O1 testcase.c
$ ./a.out 
Aborted

I think the code should have defined behavior at least at -O1 or with
-fno-strict-aliasing.

$ x86_64-pc-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r15-1122-20240609121028-g8bb6b2f4ae1-checking-yes-rtl-df-extra-nobootstrap-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/15.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--disable-bootstrap --with-cloog --with-ppl --with-isl
--build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu
--target=x86_64-pc-linux-gnu --with-ld=/usr/bin/x86_64-pc-linux-gnu-ld
--with-as=/usr/bin/x86_64-pc-linux-gnu-as --enable-libsanitizer
--disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r15-1122-20240609121028-g8bb6b2f4ae1-checking-yes-rtl-df-extra-nobootstrap-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 15.0.0 20240609 (experimental) (GCC)

[Bug lto/112732] during IPA pass: *free_lang_data ICE: 'verify_type' failed: type variant with 'TYPE_ALIAS_SET_KNOWN_P' with -Os -Wstrict-aliasing=2 -flto

2024-06-04 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112732

--- Comment #6 from GCC Commits  ---
The releases/gcc-12 branch has been updated by Richard Biener
:

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

commit r12-10492-gb46486ef0316240eb3c173bda062b52333507e03
Author: Richard Biener 
Date:   Tue Nov 28 12:36:21 2023 +0100

middle-end/112732 - stray TYPE_ALIAS_SET in type variant

The following fixes a stray TYPE_ALIAS_SET in a type variant built
by build_opaque_vector_type which is diagnosed by type checking
enabled with -flto.

PR middle-end/112732
* tree.cc (build_opaque_vector_type): Reset TYPE_ALIAS_SET
of the newly built type.

(cherry picked from commit f26d68d5d128c86faaceeb81b1e8f22254ad53df)

[gcc r15-934] C23: allow aliasing for types derived from structs with variable size

2024-05-30 Thread Martin Uecker via Gcc-cvs
https://gcc.gnu.org/g:d2cfe8a73b3c4195a25cde28e1641ef36ebb08c1

commit r15-934-gd2cfe8a73b3c4195a25cde28e1641ef36ebb08c1
Author: Martin Uecker 
Date:   Fri May 24 12:35:27 2024 +0200

C23: allow aliasing for types derived from structs with variable size

Previously, we set the aliasing set of structures with variable size

struct foo { int x[n]; char b; };

to zero. The reason is that such types can be compatible to diffrent
structure types which are incompatible.

struct foo { int x[2]; char b; };
struct foo { int x[3]; char b; };

But it is not enough to set the aliasing set to zero, because derived
types would then still end up in different equivalence classes even
though they might be compatible.  Instead those types should be set
to structural equivalency.  We also add checking assertions that
ensure that TYPE_CANONICAL is set correctly for all tagged types.

gcc/c/
* c-decl.cc (finish_struct): Do not set TYPE_CANONICAL for
structure or unions with variable size.
* c-objc-common.cc (c_get_alias_set): Do not set alias set to zero.
* c-typeck.cc (comptypes_verify): New function.
(comptypes,comptypes_same_p,comptypes_check_enum_int): Add 
assertion.
(comptypes_equiv_p): Add assertion that ensures that compatible
types have the same equivalence class.
(tagged_types_tu_compatible_p): Remove now unneeded special case.

gcc/testsuite/
* gcc.dg/gnu23-tag-alias-8.c: New test.

Diff:
---
 gcc/c/c-decl.cc  |  2 +-
 gcc/c/c-objc-common.cc   |  5 -
 gcc/c/c-typeck.cc| 37 +---
 gcc/testsuite/gcc.dg/gnu23-tag-alias-8.c | 24 +
 4 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 6e6606c9570..9f7d55c0b10 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9749,7 +9749,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, 
tree attributes,
   C_TYPE_BEING_DEFINED (t) = 0;
 
   /* Set type canonical based on equivalence class.  */
-  if (flag_isoc23)
+  if (flag_isoc23 && !C_TYPE_VARIABLE_SIZE (t))
 {
   if (c_struct_htab == NULL)
c_struct_htab = hash_table::create_ggc (61);
diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc
index 283f6a8ae26..738e899a2a9 100644
--- a/gcc/c/c-objc-common.cc
+++ b/gcc/c/c-objc-common.cc
@@ -420,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED)
 alias_set_type
 c_get_alias_set (tree t)
 {
-  /* Structs with variable size can alias different incompatible
- structs.  Let them alias anything.   */
-  if (RECORD_OR_UNION_TYPE_P (t) && C_TYPE_VARIABLE_SIZE (t))
-return 0;
-
   return c_common_get_alias_set (t);
 }
 
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 09b2c265a46..48934802148 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -1167,6 +1167,28 @@ common_type (tree t1, tree t2)
   return c_common_type (t1, t2);
 }
 
+
+
+/* Helper function for comptypes.  For two compatible types, return 1
+   if they pass consistency checks.  In particular we test that
+   TYPE_CANONICAL is set correctly, i.e. the two types can alias.  */
+
+static bool
+comptypes_verify (tree type1, tree type2)
+{
+  if (TYPE_CANONICAL (type1) != TYPE_CANONICAL (type2)
+  && !TYPE_STRUCTURAL_EQUALITY_P (type1)
+  && !TYPE_STRUCTURAL_EQUALITY_P (type2))
+{
+  /* FIXME: check other types. */
+  if (RECORD_OR_UNION_TYPE_P (type1)
+ || TREE_CODE (type1) == ENUMERAL_TYPE
+ || TREE_CODE (type2) == ENUMERAL_TYPE)
+   return false;
+}
+  return true;
+}
+
 struct comptypes_data {
   bool enum_and_int_p;
   bool different_types_p;
@@ -1188,6 +1210,8 @@ comptypes (tree type1, tree type2)
   struct comptypes_data data = { };
   bool ret = comptypes_internal (type1, type2, );
 
+  gcc_checking_assert (!ret || comptypes_verify (type1, type2));
+
   return ret ? (data.warning_needed ? 2 : 1) : 0;
 }
 
@@ -1201,6 +1225,8 @@ comptypes_same_p (tree type1, tree type2)
   struct comptypes_data data = { };
   bool ret = comptypes_internal (type1, type2, );
 
+  gcc_checking_assert (!ret || comptypes_verify (type1, type2));
+
   if (data.different_types_p)
 return false;
 
@@ -1218,6 +1244,8 @@ comptypes_check_enum_int (tree type1, tree type2, bool 
*enum_and_int_p)
   bool ret = comptypes_internal (type1, type2, );
   *enum_and_int_p = data.enum_and_int_p;
 
+  gcc_checking_assert (!ret || comptypes_verify (type1, type2));
+
   return ret ? (data.warning_needed ? 2 : 1) : 0;
 }
 
@@ -1232,6 +1260,8 @@ comptypes_check_different_types (tree type1, tree type2,
   bool ret = comptypes_internal (type1, type2, );
   *different_types_p = data.different_types_p;
 
+  gcc_checking_assert (!ret || comptypes_verify (type1, type2));
+
   retur

[gcc r15-933] C: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-30 Thread Martin Uecker via Gcc-cvs
https://gcc.gnu.org/g:867d1264fe71d4291194373d1a1c409cac97a597

commit r15-933-g867d1264fe71d4291194373d1a1c409cac97a597
Author: Martin Uecker 
Date:   Sun May 19 23:13:22 2024 +0200

C: allow aliasing of compatible types derived from enumeral types [PR115157]

Aliasing of enumeral types with the underlying integer is now allowed
by setting the aliasing set to zero.  But this does not allow aliasing
of derived types which are compatible as required by ISO C.  Instead,
initially set structural equality.  Then set TYPE_CANONICAL and update
pointers and main variants when the type is completed (as done for
structures and unions in C23).

PR tree-optimization/115157
PR tree-optimization/115177

gcc/c/
* c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL.
* c-objc-common.cc (get_alias_set): Remove special case.
(get_aka_type): Add special case.

gcc/c-family/
* c-attribs.cc (handle_hardbool_attribute): Set TYPE_CANONICAL
for hardbools.

gcc/
* godump.cc (go_output_typedef): Use TYPE_MAIN_VARIANT instead
of TYPE_CANONICAL.

gcc/testsuite/
* gcc.dg/enum-alias-1.c: New test.
* gcc.dg/enum-alias-2.c: New test.
* gcc.dg/enum-alias-3.c: New test.
* gcc.dg/enum-alias-4.c: New test.

Diff:
---
 gcc/c-family/c-attribs.cc   |  1 +
 gcc/c/c-decl.cc | 11 +--
 gcc/c/c-objc-common.cc  |  7 ++-
 gcc/godump.cc   | 10 +++---
 gcc/testsuite/gcc.dg/enum-alias-1.c | 24 
 gcc/testsuite/gcc.dg/enum-alias-2.c | 25 +
 gcc/testsuite/gcc.dg/enum-alias-3.c | 26 ++
 gcc/testsuite/gcc.dg/enum-alias-4.c | 22 ++
 8 files changed, 112 insertions(+), 14 deletions(-)

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 605469dd7dd..e3833ed5f20 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -1074,6 +1074,7 @@ handle_hardbool_attribute (tree *node, tree name, tree 
args,
 
   TREE_SET_CODE (*node, ENUMERAL_TYPE);
   ENUM_UNDERLYING_TYPE (*node) = orig;
+  TYPE_CANONICAL (*node) = TYPE_CANONICAL (orig);
 
   tree false_value;
   if (args)
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index b691b91b3db..6e6606c9570 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, 
int warned)
  if (t == NULL_TREE)
{
  t = make_node (code);
- if (flag_isoc23 && code != ENUMERAL_TYPE)
+ if (flag_isoc23 || code == ENUMERAL_TYPE)
SET_TYPE_STRUCTURAL_EQUALITY (t);
  pushtag (input_location, name, t);
}
@@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, 
tree name,
  the forward-reference will be altered into a real type.  */
 
   ref = make_node (code);
-  if (flag_isoc23 && code != ENUMERAL_TYPE)
+  if (flag_isoc23 || code == ENUMERAL_TYPE)
 SET_TYPE_STRUCTURAL_EQUALITY (ref);
   if (code == ENUMERAL_TYPE)
 {
@@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents 
*the_enum, tree name,
 {
   enumtype = make_node (ENUMERAL_TYPE);
   TYPE_SIZE (enumtype) = NULL_TREE;
+  SET_TYPE_STRUCTURAL_EQUALITY (enumtype);
   pushtag (loc, name, enumtype);
   if (fixed_underlying_type != NULL_TREE)
{
@@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents 
*the_enum, tree name,
  TYPE_SIZE (enumtype) = NULL_TREE;
  TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type);
  ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type;
+ TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type);
+ c_update_type_canonical (enumtype);
  layout_type (enumtype);
}
 }
@@ -10094,6 +10097,10 @@ finish_enum (tree enumtype, tree values, tree 
attributes)
   ENUM_UNDERLYING_TYPE (enumtype) =
c_common_type_for_size (TYPE_PRECISION (tem), TYPE_UNSIGNED (tem));
 
+  TYPE_CANONICAL (enumtype) =
+   TYPE_CANONICAL (ENUM_UNDERLYING_TYPE (enumtype));
+  c_update_type_canonical (enumtype);
+
   layout_type (enumtype);
 }
 
diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc
index 42a62c84fe7..283f6a8ae26 100644
--- a/gcc/c/c-objc-common.cc
+++ b/gcc/c/c-objc-common.cc
@@ -130,6 +130,8 @@ get_aka_type (tree type)
 
   result = get_aka_type (orig_type);
 }
+  else if (TREE_CODE (type) == ENUMERAL_TYPE)
+return type;
   else
 {
   tree canonical = TYPE_CANONICAL (type);
@@ -418,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED)

Re: [C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-30 Thread Ian Lance Taylor
On Thu, May 30, 2024 at 12:48 AM Martin Uecker  wrote:
>
>
> Hi Ian,
>
> can you give me a green light for the go changes. The C FE
> changes were approved.
>
> The only change with respect to the last version are
> the removal of the unneeded null check for the
> main variant (as discussed) and that I also removed the
>
> container->decls_seen.add (TREE_TYPE (decl));
>
> and the corresponding check, because I think it is
> redundant if we also test for the main variant.
> (it wasn't with TYPE_CANONICAL because this was
> only added conditionally).
>
> The other code in the file only checks for added
> declarations not types, so should not depend on this.

Apologies.  I thought that I had already said that the Go changes are
fine if the libgo tests still pass.  Anyhow, that is the case: if the
tests pass, the change is fine.  Thanks.

Ian


Re: [C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-30 Thread Martin Uecker


Hi Ian,

can you give me a green light for the go changes. The C FE
changes were approved.

The only change with respect to the last version are
the removal of the unneeded null check for the
main variant (as discussed) and that I also removed the

container->decls_seen.add (TREE_TYPE (decl));

and the corresponding check, because I think it is
redundant if we also test for the main variant.
(it wasn't with TYPE_CANONICAL because this was
only added conditionally).

The other code in the file only checks for added
declarations not types, so should not depend on this.

Martin


Am Freitag, dem 24.05.2024 um 17:39 +0200 schrieb Martin Uecker:
> This is another version of this patch with two changes:
> 
> - I added a fix (with test) for PR 115177 which is just the same
> issue for hardbools which are internally implemented as enums.
> 
> - I fixed the golang issue. Since the addition of the main variant
> to the seen decls is unconditional I removed also the addition
> of the type itself which now seems unnecessary.
> 
> Bootstrapped and regression tested on x86_64.
> 
> Martin
> 
> 
> 
> C: allow aliasing of compatible types derived from enumeral types 
> [PR115157]
> 
> Aliasing of enumeral types with the underlying integer is now allowed
> by setting the aliasing set to zero.  But this does not allow aliasing
> of derived types which are compatible as required by ISO C.  Instead,
> initially set structural equality.  Then set TYPE_CANONICAL and update
> pointers and main variants when the type is completed (as done for
> structures and unions in C23).
> 
> PR 115157
> PR 115177
> 
> gcc/c/
> * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
> finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL.
> * c-obj-common.cc (get_alias_set): Remove special case.
> (get_aka_type): Add special case.
> 
> gcc/c-family/
> * c-attribs.cc (handle_hardbool_attribute): Set TYPE_CANONICAL
> for hardbools.
> 
> gcc/
> * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead
> of TYPE_CANONICAL.
> 
> gcc/testsuite/
> * gcc.dg/enum-alias-1.c: New test.
> * gcc.dg/enum-alias-2.c: New test.
> * gcc.dg/enum-alias-3.c: New test.
> * gcc.dg/enum-alias-4.c: New test.
> 
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 04e39b41bdf..033395093b6 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -1074,6 +1074,7 @@ handle_hardbool_attribute (tree *node, tree name, tree 
> args,
>  
>TREE_SET_CODE (*node, ENUMERAL_TYPE);
>ENUM_UNDERLYING_TYPE (*node) = orig;
> +  TYPE_CANONICAL (*node) = TYPE_CANONICAL (orig);
>  
>tree false_value;
>if (args)
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index b691b91b3db..6e6606c9570 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, 
> int warned)
> if (t == NULL_TREE)
>   {
> t = make_node (code);
> -   if (flag_isoc23 && code != ENUMERAL_TYPE)
> +   if (flag_isoc23 || code == ENUMERAL_TYPE)
>   SET_TYPE_STRUCTURAL_EQUALITY (t);
> pushtag (input_location, name, t);
>   }
> @@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, 
> tree name,
>   the forward-reference will be altered into a real type.  */
>  
>ref = make_node (code);
> -  if (flag_isoc23 && code != ENUMERAL_TYPE)
> +  if (flag_isoc23 || code == ENUMERAL_TYPE)
>  SET_TYPE_STRUCTURAL_EQUALITY (ref);
>if (code == ENUMERAL_TYPE)
>  {
> @@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents 
> *the_enum, tree name,
>  {
>enumtype = make_node (ENUMERAL_TYPE);
>TYPE_SIZE (enumtype) = NULL_TREE;
> +  SET_TYPE_STRUCTURAL_EQUALITY (enumtype);
>pushtag (loc, name, enumtype);
>if (fixed_underlying_type != NULL_TREE)
>   {
> @@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents 
> *the_enum, tree name,
> TYPE_SIZE (enumtype) = NULL_TREE;
> TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type);
> ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type;
> +   TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type);
> +   c_update_type_canonical (enumtype);
> layout_type (enumtype);
>   }
>  }
> @@ -10094,6 

[gcc r15-912] C23: fix aliasing for structures/unions with incomplete types

2024-05-29 Thread Martin Uecker via Gcc-cvs
https://gcc.gnu.org/g:86b98d939989427ff025bcfd536ad361fcdc699c

commit r15-912-g86b98d939989427ff025bcfd536ad361fcdc699c
Author: Martin Uecker 
Date:   Sat Mar 30 19:49:48 2024 +0100

C23: fix aliasing for structures/unions with incomplete types

When incomplete structure/union types are completed later, compatibility
of struct types that contain pointers to such types changes.  When forming
equivalence classes for TYPE_CANONICAL, we therefor need to be conservative
and treat all structs with the same tag which are pointer targets as
equivalent for purposed of determining equivalency of structure/union
types which contain such types as member. This avoids having to update
TYPE_CANONICAL of such structure/unions recursively. The pointer types
themselves are updated in c_update_type_canonical.

gcc/c/
* c-typeck.cc (comptypes_internal): Add flag to track
whether a struct is the target of a pointer.
(tagged_types_tu_compatible): When forming equivalence
classes, treat nested pointed-to structs as equivalent.

gcc/testsuite/
* gcc.dg/c23-tag-incomplete-alias-1.c: New test.

Diff:
---
 gcc/c/c-typeck.cc | 43 +--
 gcc/testsuite/gcc.dg/c23-tag-incomplete-alias-1.c | 36 +++
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index ad4c7add562..09b2c265a46 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -1172,6 +1172,7 @@ struct comptypes_data {
   bool different_types_p;
   bool warning_needed;
   bool anon_field;
+  bool pointedto;
   bool equiv;
 
   const struct tagged_tu_seen_cache* cache;
@@ -1235,8 +1236,36 @@ comptypes_check_different_types (tree type1, tree type2,
 }
 
 
-/* Like comptypes, but if it returns nonzero for struct and union
-   types considered equivalent for aliasing purposes.  */
+/* Like comptypes, but if it returns true for struct and union types
+   considered equivalent for aliasing purposes, i.e. for setting
+   TYPE_CANONICAL after completing a struct or union.
+
+   This function must return false only for types which are not
+   compatible according to C language semantics (cf. comptypes),
+   otherwise the middle-end would make incorrect aliasing decisions.
+   It may return true for some similar types that are not compatible
+   according to those stricter rules.
+
+   In particular, we ignore size expression in arrays so that the
+   following structs are in the same equivalence class:
+
+   struct foo { char (*buf)[]; };
+   struct foo { char (*buf)[3]; };
+   struct foo { char (*buf)[4]; };
+
+   We also treat unions / structs with members which are pointers to
+   structures or unions with the same tag as equivalent (if they are not
+   incompatible for other reasons).  Although incomplete structure
+   or union types are not compatible to any other type, they may become
+   compatible to different types when completed.  To avoid having to update
+   TYPE_CANONICAL at this point, we only consider the tag when forming
+   the equivalence classes.  For example, the following types with tag
+   'foo' are all considered equivalent:
+
+   struct bar;
+   struct foo { struct bar *x };
+   struct foo { struct bar { int a; } *x };
+   struct foo { struct bar { char b; } *x };  */
 
 bool
 comptypes_equiv_p (tree type1, tree type2)
@@ -1357,6 +1386,7 @@ comptypes_internal (const_tree type1, const_tree type2,
   /* Do not remove mode information.  */
   if (TYPE_MODE (t1) != TYPE_MODE (t2))
return false;
+  data->pointedto = true;
   return comptypes_internal (TREE_TYPE (t1), TREE_TYPE (t2), data);
 
 case FUNCTION_TYPE:
@@ -1375,7 +1405,7 @@ comptypes_internal (const_tree type1, const_tree type2,
 
if ((d1 == NULL_TREE) != (d2 == NULL_TREE))
  data->different_types_p = true;
-   /* Ignore size mismatches.  */
+   /* Ignore size mismatches when forming equivalence classes.  */
if (data->equiv)
  return true;
/* Sizes must match unless one is missing or variable.  */
@@ -1515,6 +1545,12 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree 
t2,
   if (TYPE_NAME (t1) != TYPE_NAME (t2))
 return false;
 
+  /* When forming equivalence classes for TYPE_CANONICAL in C23, we treat
+ structs with the same tag as equivalent, but only when they are targets
+ of pointers inside other structs.  */
+  if (data->equiv && data->pointedto)
+return true;
+
   if (!data->anon_field && NULL_TREE == TYPE_NAME (t1))
 return false;
 
@@ -1610,6 +1646,7 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree 
t2,
  return false;
 
data->anon_field = !DECL_NAME (s1);
+   data->pointedto = false;
 
data->cache = 
if (!comptypes_internal (TREE_TYPE (s1), TREE_TYPE (s2), d

Re: [C23 PATCH]: allow aliasing for types derived from structs with variable size

2024-05-28 Thread Joseph Myers
On Sun, 26 May 2024, Martin Uecker wrote:

> +/* Helper function for comptypes.  For two compatible types, return 1
> +   if they pass consistency checks.  In particular we test that
> +   TYPE_CANONICAL ist set correctly, i.e. the two types can alias.  */

s/ist/is/.  OK with that fix.

-- 
Joseph S. Myers
josmy...@redhat.com



Re: [C23 PATCH, v2] fix aliasing for structures/unions with incomplete types

2024-05-28 Thread Joseph Myers
On Sun, 26 May 2024, Martin Uecker wrote:

> This is the patch I sent previously, but I tried to improve the
> description and added a long comment.  This patch is needed so
> that we do not have to update TYPE_CANONICAL of structures / unions
> when a tagged type is completed that is (recursively) pointed to 
> by a member of the structure / union.
> 
> Bootstrapped and regression tested on x86_64.
> 
> 
> C23: fix aliasing for structures/unions with incomplete types
> 
> When incomplete structure/union types are completed later, compatibility
> of struct types that contain pointers to such types changes.  When forming
> equivalence classes for TYPE_CANONICAL, we therefor need to be 
> conservative
> and treat all structs with the same tag which are pointer targets as
> equivalent for purposed of determining equivalency of structure/union
> types which contain such types as member. This avoids having to update
> TYPE_CANONICAL of such structure/unions recursively. The pointer types
> themselves are updated in c_update_type_canonical.
> 
> gcc/c/
> * c-typeck.cc (comptypes_internal): Add flag to track
> whether a struct is the target of a pointer.
> (tagged_types_tu_compatible): When forming equivalence
> classes, treat nested pointed-to structs as equivalent.
> 
> gcc/testsuite/
> * gcc.dg/c23-tag-incomplete-alias-1.c: New test.

This patch is OK.

-- 
Joseph S. Myers
josmy...@redhat.com

Re: [C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-28 Thread Joseph Myers
On Fri, 24 May 2024, Martin Uecker wrote:

> This is another version of this patch with two changes:
> 
> - I added a fix (with test) for PR 115177 which is just the same
> issue for hardbools which are internally implemented as enums.
> 
> - I fixed the golang issue. Since the addition of the main variant
> to the seen decls is unconditional I removed also the addition
> of the type itself which now seems unnecessary.
> 
> Bootstrapped and regression tested on x86_64.

The front-end changes and the testcases are OK.

-- 
Joseph S. Myers
josmy...@redhat.com



[C23 PATCH]: allow aliasing for types derived from structs with variable size

2024-05-26 Thread Martin Uecker


This is similar to the enum issue, where setting the alias set to zero
is insufficient because also derived types need to be able to alias.

After this change, it is also possible to add checking assertions that
verify TYPE_CANONICAL for all tagged types at end of each call to the
comptypes family.  At least for the test suite this then shows no
additional issues for tagged types.   

(We still make incorrect aliasing decisions at least for types derived 
from  functions types (PR114959) but this is an ancient issue.  We
probably should set TYPE_CANONICAL in the FE similar to what is done
for tagged types in C23.)


Bootstrapped and regression tested on x86_64.



C23: allow aliasing for types derived from structs with variable size

Previously, we set the aliasing set of structures with variable size

struct foo { int x[n]; char b; };

to zero. The reason is that such types can be compatible to diffrent
structure types which are incompatible.

struct foo { int x[2]; char b; };
struct foo { int x[3]; char b; };

But it is not enough to set the aliasing set to zero, because derived
types would then still end up in different equivalence classes even
though they might be compatible.  Instead those types should be set
to structural equivalency.  We also add checking assertions that
ensure that TYPE_CANONICAL is set correctly for all tagged types

gcc/c/
* c-decl.cc (finish_struct): Do not set TYPE_CANONICAL for
structure or unions with variable size.
* c-objc-common.cc (c_get_alias_set): Do not set alias set to zero.
* c-typeck.cc (comptypes_verify): New function.
(comptypes,comptypes_same_p,comptypes_check_enum_int): Add 
assertion.
(comptypes_equiv_p): Add assertion that ensures that compatible
types have the same equivalence class.
(tagged_types_tu_compatible_p): Remove now unneeded special case.

gcc/testsuite/
* gcc.dg/gnu23-tag-alias-8.c: New test.

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 6e6606c9570..9f7d55c0b10 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9749,7 +9749,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, 
tree attributes,
   C_TYPE_BEING_DEFINED (t) = 0;
 
   /* Set type canonical based on equivalence class.  */
-  if (flag_isoc23)
+  if (flag_isoc23 && !C_TYPE_VARIABLE_SIZE (t))
 {
   if (c_struct_htab == NULL)
c_struct_htab = hash_table::create_ggc (61);
diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc
index 551ec6f4b65..29127d037e1 100644
--- a/gcc/c/c-objc-common.cc
+++ b/gcc/c/c-objc-common.cc
@@ -420,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED)
 alias_set_type
 c_get_alias_set (tree t)
 {
-  /* Structs with variable size can alias different incompatible
- structs.  Let them alias anything.   */
-  if (RECORD_OR_UNION_TYPE_P (t) && C_TYPE_VARIABLE_SIZE (t))
-return 0;
-
   return c_common_get_alias_set (t);
 }
 
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 2d092357e0f..c07e2f2b5cf 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -1167,6 +1167,28 @@ common_type (tree t1, tree t2)
   return c_common_type (t1, t2);
 }
 
+
+
+/* Helper function for comptypes.  For two compatible types, return 1
+   if they pass consistency checks.  In particular we test that
+   TYPE_CANONICAL ist set correctly, i.e. the two types can alias.  */
+
+static bool
+comptypes_verify (tree type1, tree type2)
+{
+  if (TYPE_CANONICAL (type1) != TYPE_CANONICAL (type2)
+  && !TYPE_STRUCTURAL_EQUALITY_P (type1)
+  && !TYPE_STRUCTURAL_EQUALITY_P (type2))
+{
+  /* FIXME: check other types. */
+  if (RECORD_OR_UNION_TYPE_P (type1)
+ || TREE_CODE (type1) == ENUMERAL_TYPE
+ || TREE_CODE (type2) == ENUMERAL_TYPE)
+   return false;
+}
+  return true;
+}
+
 struct comptypes_data {
   bool enum_and_int_p;
   bool different_types_p;
@@ -1187,6 +1209,8 @@ comptypes (tree type1, tree type2)
   struct comptypes_data data = { };
   bool ret = comptypes_internal (type1, type2, );
 
+  gcc_checking_assert (!ret || comptypes_verify (type1, type2));
+
   return ret ? (data.warning_needed ? 2 : 1) : 0;
 }
 
@@ -1200,6 +1224,8 @@ comptypes_same_p (tree type1, tree type2)
   struct comptypes_data data = { };
   bool ret = comptypes_internal (type1, type2, );
 
+  gcc_checking_assert (!ret || comptypes_verify (type1, type2));
+
   if (data.different_types_p)
 return false;
 
@@ -1217,6 +1243,8 @@ comptypes_check_enum_int (tree type1, tree type2, bool 
*enum_and_int_p)
   bool ret = comptypes_internal (type1, type2, );
   *enum_and_int_p = data.enum_and_int_p;
 
+  gcc_checking_assert (!ret || comptypes_verify (type1, type2));
+
   return ret ? (data.warning_needed ? 2 : 1) : 0;
 }
 
@@ -1231,6 +1259,8 @@ comptypes_check_different_types (tree type1, tree type2,

[C23 PATCH, v2] fix aliasing for structures/unions with incomplete types

2024-05-26 Thread Martin Uecker


This is the patch I sent previously, but I tried to improve the
description and added a long comment.  This patch is needed so
that we do not have to update TYPE_CANONICAL of structures / unions
when a tagged type is completed that is (recursively) pointed to 
by a member of the structure / union.

Bootstrapped and regression tested on x86_64.


C23: fix aliasing for structures/unions with incomplete types

When incomplete structure/union types are completed later, compatibility
of struct types that contain pointers to such types changes.  When forming
equivalence classes for TYPE_CANONICAL, we therefor need to be conservative
and treat all structs with the same tag which are pointer targets as
equivalent for purposed of determining equivalency of structure/union
types which contain such types as member. This avoids having to update
TYPE_CANONICAL of such structure/unions recursively. The pointer types
themselves are updated in c_update_type_canonical.

gcc/c/
* c-typeck.cc (comptypes_internal): Add flag to track
whether a struct is the target of a pointer.
(tagged_types_tu_compatible): When forming equivalence
classes, treat nested pointed-to structs as equivalent.

gcc/testsuite/
* gcc.dg/c23-tag-incomplete-alias-1.c: New test.

diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index c07e2f2b5cf..7a14ef1868f 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -1194,6 +1194,7 @@ struct comptypes_data {
   bool different_types_p;
   bool warning_needed;
   bool anon_field;
+  bool pointedto;
   bool equiv;
 
   const struct tagged_tu_seen_cache* cache;
@@ -1265,8 +1266,36 @@ comptypes_check_different_types (tree type1, tree type2,
 }
 
 
-/* Like comptypes, but if it returns nonzero for struct and union
-   types considered equivalent for aliasing purposes.  */
+/* Like comptypes, but if it returns true for struct and union types
+   considered equivalent for aliasing purposes, i.e. for setting
+   TYPE_CANONICAL after completing a struct or union.
+
+   This function must return false only for types which are not
+   compatible according to C language semantics (cf. comptypes),
+   otherwise the middle-end would make incorrect aliasing decisions.
+   It may return true for some similar types that are not compatible
+   according to those stricter rules.
+
+   In particular, we ignore size expression in arrays so that the
+   following structs are in the same equivalence class:
+
+   struct foo { char (*buf)[]; };
+   struct foo { char (*buf)[3]; };
+   struct foo { char (*buf)[4]; };
+
+   We also treat unions / structs with members which are pointers to
+   structures or unions with the same tag as equivalent (if they are not
+   incompatible for other reasons).  Although incomplete structure
+   or union types are not compatible to any other type, they may become
+   compatible to different types when completed.  To avoid having to update
+   TYPE_CANONICAL at this point, we only consider the tag when forming
+   the equivalence classes.  For example, the following types with tag
+   'foo' are all considered equivalent:
+
+   struct bar;
+   struct foo { struct bar *x };
+   struct foo { struct bar { int a; } *x };
+   struct foo { struct bar { char b; } *x };  */
 
 bool
 comptypes_equiv_p (tree type1, tree type2)
@@ -1391,6 +1420,7 @@ comptypes_internal (const_tree type1, const_tree type2,
   /* Do not remove mode information.  */
   if (TYPE_MODE (t1) != TYPE_MODE (t2))
return false;
+  data->pointedto = true;
   return comptypes_internal (TREE_TYPE (t1), TREE_TYPE (t2), data);
 
 case FUNCTION_TYPE:
@@ -1409,7 +1439,7 @@ comptypes_internal (const_tree type1, const_tree type2,
 
if ((d1 == NULL_TREE) != (d2 == NULL_TREE))
  data->different_types_p = true;
-   /* Ignore size mismatches.  */
+   /* Ignore size mismatches when forming equivalence classes.  */
if (data->equiv)
  return true;
/* Sizes must match unless one is missing or variable.  */
@@ -1549,6 +1579,12 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree 
t2,
   if (TYPE_NAME (t1) != TYPE_NAME (t2))
 return false;
 
+  /* When forming equivalence classes for TYPE_CANONICAL in C23, we treat
+ structs with the same tag as equivalent, but only when they are targets
+ of pointers inside other structs.  */
+  if (data->equiv && data->pointedto)
+return true;
+
   if (!data->anon_field && NULL_TREE == TYPE_NAME (t1))
 return false;
 
@@ -1641,6 +1677,7 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree 
t2,
  return false;
 
data->anon_field = !DECL_NAME (s1);
+   data->pointedto = false;
 
data->cache = 
if (!comptypes_internal (TREE_TYPE (s1), TREE_TYPE (s2), data))
diff --git a/gcc/testsuite/gcc.dg/c23-tag-incom

Re: [C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-24 Thread Jakub Jelinek
On Fri, May 24, 2024 at 05:39:45PM +0200, Martin Uecker wrote:
> PR 115157
> PR 115177
> 
> gcc/c/
> * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
> finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL.
> * c-obj-common.cc (get_alias_set): Remove special case.
> (get_aka_type): Add special case.
> 
> gcc/c-family/
> * c-attribs.cc (handle_hardbool_attribute): Set TYPE_CANONICAL
> for hardbools.
> 
> gcc/
> * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead
> of TYPE_CANONICAL.

Just a nit:
s/use/Use/

Jakub



[C PATCH, v2]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-24 Thread Martin Uecker


This is another version of this patch with two changes:

- I added a fix (with test) for PR 115177 which is just the same
issue for hardbools which are internally implemented as enums.

- I fixed the golang issue. Since the addition of the main variant
to the seen decls is unconditional I removed also the addition
of the type itself which now seems unnecessary.

Bootstrapped and regression tested on x86_64.

Martin



C: allow aliasing of compatible types derived from enumeral types [PR115157]

Aliasing of enumeral types with the underlying integer is now allowed
by setting the aliasing set to zero.  But this does not allow aliasing
of derived types which are compatible as required by ISO C.  Instead,
initially set structural equality.  Then set TYPE_CANONICAL and update
pointers and main variants when the type is completed (as done for
structures and unions in C23).

PR 115157
PR 115177

gcc/c/
* c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL.
* c-obj-common.cc (get_alias_set): Remove special case.
(get_aka_type): Add special case.

gcc/c-family/
* c-attribs.cc (handle_hardbool_attribute): Set TYPE_CANONICAL
for hardbools.

gcc/
* godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead
of TYPE_CANONICAL.

gcc/testsuite/
* gcc.dg/enum-alias-1.c: New test.
* gcc.dg/enum-alias-2.c: New test.
* gcc.dg/enum-alias-3.c: New test.
* gcc.dg/enum-alias-4.c: New test.

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 04e39b41bdf..033395093b6 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -1074,6 +1074,7 @@ handle_hardbool_attribute (tree *node, tree name, tree 
args,
 
   TREE_SET_CODE (*node, ENUMERAL_TYPE);
   ENUM_UNDERLYING_TYPE (*node) = orig;
+  TYPE_CANONICAL (*node) = TYPE_CANONICAL (orig);
 
   tree false_value;
   if (args)
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index b691b91b3db..6e6606c9570 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, 
int warned)
  if (t == NULL_TREE)
{
  t = make_node (code);
- if (flag_isoc23 && code != ENUMERAL_TYPE)
+ if (flag_isoc23 || code == ENUMERAL_TYPE)
SET_TYPE_STRUCTURAL_EQUALITY (t);
  pushtag (input_location, name, t);
}
@@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, 
tree name,
  the forward-reference will be altered into a real type.  */
 
   ref = make_node (code);
-  if (flag_isoc23 && code != ENUMERAL_TYPE)
+  if (flag_isoc23 || code == ENUMERAL_TYPE)
 SET_TYPE_STRUCTURAL_EQUALITY (ref);
   if (code == ENUMERAL_TYPE)
 {
@@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents 
*the_enum, tree name,
 {
   enumtype = make_node (ENUMERAL_TYPE);
   TYPE_SIZE (enumtype) = NULL_TREE;
+  SET_TYPE_STRUCTURAL_EQUALITY (enumtype);
   pushtag (loc, name, enumtype);
   if (fixed_underlying_type != NULL_TREE)
{
@@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents 
*the_enum, tree name,
  TYPE_SIZE (enumtype) = NULL_TREE;
  TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type);
  ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type;
+ TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type);
+ c_update_type_canonical (enumtype);
  layout_type (enumtype);
}
 }
@@ -10094,6 +10097,10 @@ finish_enum (tree enumtype, tree values, tree 
attributes)
   ENUM_UNDERLYING_TYPE (enumtype) =
c_common_type_for_size (TYPE_PRECISION (tem), TYPE_UNSIGNED (tem));
 
+  TYPE_CANONICAL (enumtype) =
+   TYPE_CANONICAL (ENUM_UNDERLYING_TYPE (enumtype));
+  c_update_type_canonical (enumtype);
+
   layout_type (enumtype);
 }
 
diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc
index b7c72d2609c..551ec6f4b65 100644
--- a/gcc/c/c-objc-common.cc
+++ b/gcc/c/c-objc-common.cc
@@ -130,6 +130,8 @@ get_aka_type (tree type)
 
   result = get_aka_type (orig_type);
 }
+  else if (TREE_CODE (type) == ENUMERAL_TYPE)
+return type;
   else
 {
   tree canonical = TYPE_CANONICAL (type);
@@ -418,11 +420,6 @@ c_var_mod_p (tree x, tree fn ATTRIBUTE_UNUSED)
 alias_set_type
 c_get_alias_set (tree t)
 {
-  /* Allow aliasing between enumeral types and the underlying
- integer type.  This is required since those are compatible types.  */
-  if (TREE_CODE (t) == ENUMERAL_TYPE)
-return get_alias_set (ENUM_UNDERLYING_TYPE (t));
-
   /* Structs with variable size can alias different incompatible
   

Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-23 Thread Richard Biener
On Thu, 23 May 2024, Ian Lance Taylor wrote:

> On Thu, May 23, 2024 at 2:48 PM Martin Uecker  wrote:
> >
> > Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor:
> > > On Thu, May 23, 2024 at 2:00 PM Joseph Myers  wrote:
> > > >
> > > > On Tue, 21 May 2024, Martin Uecker wrote:
> > > > >
> > > > > C: allow aliasing of compatible types derived from enumeral types 
> > > > > [PR115157]
> > > > >
> > > > > Aliasing of enumeral types with the underlying integer is now 
> > > > > allowed
> > > > > by setting the aliasing set to zero.  But this does not allow 
> > > > > aliasing
> > > > > of derived types which are compatible as required by ISO C.  
> > > > > Instead,
> > > > > initially set structural equality.  Then set TYPE_CANONICAL and 
> > > > > update
> > > > > pointers and main variants when the type is completed (as done for
> > > > > structures and unions in C23).
> > > > >
> > > > > PR 115157
> > > > >
> > > > > gcc/c/
> > > > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
> > > > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / 
> > > > > TYPE_CANONICAL.
> > > > > * c-obj-common.cc (get_alias_set): Remove special case.
> > > > > (get_aka_type): Add special case.
> > > > >
> > > > > gcc/
> > > > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT 
> > > > > instead
> > > > > of TYPE_CANONICAL.
> > > > >
> > > > > gcc/testsuite/
> > > > > * gcc.dg/enum-alias-1.c: New test.
> > > > > * gcc.dg/enum-alias-2.c: New test.
> > > > > * gcc.dg/enum-alias-3.c: New test.
> > > >
> > > > OK, in the absence of objections on middle-end or Go grounds within the
> > > > next week.
> > >
> > > The godump.cc patch is
> > >
> > >&& (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE
> > >   || !container->decls_seen.contains
> > > -   (TYPE_CANONICAL (TREE_TYPE (decl)
> > > +   (TYPE_MAIN_VARIANT (TREE_TYPE 
> > > (decl)
> > >  {
> > >
> > > What is the problem you are seeing?
> >
> > Test failures in godump-1.c
> >
> > >
> > > This patch isn't right:
> > >
> > > 1) The code is saying if "X == NULL_TREE || !already_seen(X)".  This
> > > patch is changing the latter X but not the former.  They should be
> > > consistent.
> >
> > Maybe the X == NULL_TREE can be removed if we
> > add TYPE_MAIN_VARIANTs instead?
> 
> If TYPE_MAIN_VARIANT is never NULL_TREE, then I agree that the
> NULL_TREE test can be removed.

TYPE_MAIN_VARIANT is indeed never NULL_TREE.

Richard.

Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-23 Thread Ian Lance Taylor
On Thu, May 23, 2024 at 2:48 PM Martin Uecker  wrote:
>
> Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor:
> > On Thu, May 23, 2024 at 2:00 PM Joseph Myers  wrote:
> > >
> > > On Tue, 21 May 2024, Martin Uecker wrote:
> > > >
> > > > C: allow aliasing of compatible types derived from enumeral types 
> > > > [PR115157]
> > > >
> > > > Aliasing of enumeral types with the underlying integer is now 
> > > > allowed
> > > > by setting the aliasing set to zero.  But this does not allow 
> > > > aliasing
> > > > of derived types which are compatible as required by ISO C.  
> > > > Instead,
> > > > initially set structural equality.  Then set TYPE_CANONICAL and 
> > > > update
> > > > pointers and main variants when the type is completed (as done for
> > > > structures and unions in C23).
> > > >
> > > > PR 115157
> > > >
> > > > gcc/c/
> > > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
> > > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / 
> > > > TYPE_CANONICAL.
> > > > * c-obj-common.cc (get_alias_set): Remove special case.
> > > > (get_aka_type): Add special case.
> > > >
> > > > gcc/
> > > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT 
> > > > instead
> > > > of TYPE_CANONICAL.
> > > >
> > > > gcc/testsuite/
> > > > * gcc.dg/enum-alias-1.c: New test.
> > > > * gcc.dg/enum-alias-2.c: New test.
> > > > * gcc.dg/enum-alias-3.c: New test.
> > >
> > > OK, in the absence of objections on middle-end or Go grounds within the
> > > next week.
> >
> > The godump.cc patch is
> >
> >&& (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE
> >   || !container->decls_seen.contains
> > -   (TYPE_CANONICAL (TREE_TYPE (decl)
> > +   (TYPE_MAIN_VARIANT (TREE_TYPE (decl)
> >  {
> >
> > What is the problem you are seeing?
>
> Test failures in godump-1.c
>
> >
> > This patch isn't right:
> >
> > 1) The code is saying if "X == NULL_TREE || !already_seen(X)".  This
> > patch is changing the latter X but not the former.  They should be
> > consistent.
>
> Maybe the X == NULL_TREE can be removed if we
> add TYPE_MAIN_VARIANTs instead?

If TYPE_MAIN_VARIANT is never NULL_TREE, then I agree that the
NULL_TREE test can be removed.

Ian


Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-23 Thread Martin Uecker
Am Donnerstag, dem 23.05.2024 um 14:30 -0700 schrieb Ian Lance Taylor:
> On Thu, May 23, 2024 at 2:00 PM Joseph Myers  wrote:
> > 
> > On Tue, 21 May 2024, Martin Uecker wrote:
> > > 
> > > C: allow aliasing of compatible types derived from enumeral types 
> > > [PR115157]
> > > 
> > > Aliasing of enumeral types with the underlying integer is now allowed
> > > by setting the aliasing set to zero.  But this does not allow aliasing
> > > of derived types which are compatible as required by ISO C.  Instead,
> > > initially set structural equality.  Then set TYPE_CANONICAL and update
> > > pointers and main variants when the type is completed (as done for
> > > structures and unions in C23).
> > > 
> > > PR 115157
> > > 
> > > gcc/c/
> > > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
> > > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / 
> > > TYPE_CANONICAL.
> > > * c-obj-common.cc (get_alias_set): Remove special case.
> > > (get_aka_type): Add special case.
> > > 
> > > gcc/
> > > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead
> > > of TYPE_CANONICAL.
> > > 
> > > gcc/testsuite/
> > > * gcc.dg/enum-alias-1.c: New test.
> > > * gcc.dg/enum-alias-2.c: New test.
> > > * gcc.dg/enum-alias-3.c: New test.
> > 
> > OK, in the absence of objections on middle-end or Go grounds within the
> > next week.
> 
> The godump.cc patch is
> 
>&& (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE
>   || !container->decls_seen.contains
> -   (TYPE_CANONICAL (TREE_TYPE (decl)
> +   (TYPE_MAIN_VARIANT (TREE_TYPE (decl)
>  {
> 
> What is the problem you are seeing?

Test failures in godump-1.c

> 
> This patch isn't right:
> 
> 1) The code is saying if "X == NULL_TREE || !already_seen(X)".  This
> patch is changing the latter X but not the former.  They should be
> consistent.

Maybe the X == NULL_TREE can be removed if we
add TYPE_MAIN_VARIANTs instead?

> 
> 2) At the bottom of that conditional block is code that adds a value
> to container->decls_seen.  Today that code is adding TYPE_CANONICAL.
> If we change the condition to test TYPE_MAIN_VARIANT, then we need to
> add TYPE_MAIN_VARIANT to decls_seen.

Yes, obviously this is wrong. Thanks!

Martin
> 
> Hope that makes sense.
> 
> I don't know why the patch is required, but it's fine with those
> changes as long as the libgo tests continue to pass.


> 
> Ian



Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-23 Thread Ian Lance Taylor
On Thu, May 23, 2024 at 2:00 PM Joseph Myers  wrote:
>
> On Tue, 21 May 2024, Martin Uecker wrote:
> >
> > C: allow aliasing of compatible types derived from enumeral types 
> > [PR115157]
> >
> > Aliasing of enumeral types with the underlying integer is now allowed
> > by setting the aliasing set to zero.  But this does not allow aliasing
> > of derived types which are compatible as required by ISO C.  Instead,
> > initially set structural equality.  Then set TYPE_CANONICAL and update
> > pointers and main variants when the type is completed (as done for
> > structures and unions in C23).
> >
> > PR 115157
> >
> > gcc/c/
> > * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
> > finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL.
> > * c-obj-common.cc (get_alias_set): Remove special case.
> > (get_aka_type): Add special case.
> >
> > gcc/
> > * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead
> > of TYPE_CANONICAL.
> >
> > gcc/testsuite/
> > * gcc.dg/enum-alias-1.c: New test.
> > * gcc.dg/enum-alias-2.c: New test.
> > * gcc.dg/enum-alias-3.c: New test.
>
> OK, in the absence of objections on middle-end or Go grounds within the
> next week.

The godump.cc patch is

   && (TYPE_CANONICAL (TREE_TYPE (decl)) == NULL_TREE
  || !container->decls_seen.contains
-   (TYPE_CANONICAL (TREE_TYPE (decl)
+   (TYPE_MAIN_VARIANT (TREE_TYPE (decl)
 {

What is the problem you are seeing?

This patch isn't right:

1) The code is saying if "X == NULL_TREE || !already_seen(X)".  This
patch is changing the latter X but not the former.  They should be
consistent.

2) At the bottom of that conditional block is code that adds a value
to container->decls_seen.  Today that code is adding TYPE_CANONICAL.
If we change the condition to test TYPE_MAIN_VARIANT, then we need to
add TYPE_MAIN_VARIANT to decls_seen.

Hope that makes sense.

I don't know why the patch is required, but it's fine with those
changes as long as the libgo tests continue to pass.

Ian


Re: [C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-23 Thread Joseph Myers
On Tue, 21 May 2024, Martin Uecker wrote:
> 
> C: allow aliasing of compatible types derived from enumeral types 
> [PR115157]
> 
> Aliasing of enumeral types with the underlying integer is now allowed
>     by setting the aliasing set to zero.  But this does not allow aliasing
> of derived types which are compatible as required by ISO C.  Instead,
> initially set structural equality.  Then set TYPE_CANONICAL and update
> pointers and main variants when the type is completed (as done for
> structures and unions in C23).
> 
> PR 115157
> 
> gcc/c/
> * c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
> finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL.
> * c-obj-common.cc (get_alias_set): Remove special case.
> (get_aka_type): Add special case.
> 
> gcc/
> * godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead
> of TYPE_CANONICAL.
> 
> gcc/testsuite/
> * gcc.dg/enum-alias-1.c: New test.
> * gcc.dg/enum-alias-2.c: New test.
> * gcc.dg/enum-alias-3.c: New test.

OK, in the absence of objections on middle-end or Go grounds within the 
next week.

-- 
Joseph S. Myers
josmy...@redhat.com



[C PATCH]: allow aliasing of compatible types derived from enumeral types [PR115157]

2024-05-21 Thread Martin Uecker


For enum and integer we allow aliasing by specifically returning
via a langhook the aliasing set of the underlying type.
But this is not sufficient for derived types, i.e. pointers to
enums and pointers to compatible integers also need to have the
same aliasing set.

We also allow forward declarations of enums which is a GNU 
extension, but I think this has to work consistently too, so
we here have the same issue as in C23 with other tagged types.

The solution in this patch is similar to what we do in C23, i.e. 
we start out with structural equality and then set TYPE_CANONICAL 
to the underlying type. The only way to make the TYPE_CANONICAL 
system work with the C rules for type compatility seems to set 
TYPE_CANONICAL to the same type for all types in a compatibility
equivalence class (as compatibility is not transitive this puts
together similar types that are not compatible). This is the
underlying type in this case.  As all types in such an equivalence
class have the same representation, so this should always work 
in my opinion (but maybe there is some middle end aspects I am
still missing).


When testing, I so far only found two minor issues, i.e. when
computing the 'aka' type in diagnostics and an issue with
godump.cc (not sure I fixed this correctly).


Beyond this patch, we need also some change for function types 
in general and there are problably also some other issues related
to incomplete arrays as well  (I added some checking to 'comptypes'
to check that all types ruled compatible by the C FE also have 
either structural equality, or have the same TYPE_CANONICAL, and
this brings up some more inconsistencies).

Thoughts?


Bootstrapped and regression tested on x86_64 (only C, C++ so far).




C: allow aliasing of compatible types derived from enumeral types [PR115157]

Aliasing of enumeral types with the underlying integer is now allowed
by setting the aliasing set to zero.  But this does not allow aliasing
of derived types which are compatible as required by ISO C.  Instead,
initially set structural equality.  Then set TYPE_CANONICAL and update
pointers and main variants when the type is completed (as done for
structures and unions in C23).

PR 115157

gcc/c/
* c-decl.cc (shadow_tag-warned,parse_xref_tag,start_enum,
finish_enum): Set SET_TYPE_STRUCTURAL_EQUALITY / TYPE_CANONICAL.
* c-obj-common.cc (get_alias_set): Remove special case.
(get_aka_type): Add special case.

gcc/
* godump.cc (go_output_typedef): use TYPE_MAIN_VARIANT instead
of TYPE_CANONICAL.

gcc/testsuite/
* gcc.dg/enum-alias-1.c: New test.
* gcc.dg/enum-alias-2.c: New test.
* gcc.dg/enum-alias-3.c: New test.

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index b691b91b3db..6e6606c9570 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5051,7 +5051,7 @@ shadow_tag_warned (const struct c_declspecs *declspecs, 
int warned)
  if (t == NULL_TREE)
{
  t = make_node (code);
- if (flag_isoc23 && code != ENUMERAL_TYPE)
+ if (flag_isoc23 || code == ENUMERAL_TYPE)
SET_TYPE_STRUCTURAL_EQUALITY (t);
  pushtag (input_location, name, t);
}
@@ -8828,7 +8828,7 @@ parser_xref_tag (location_t loc, enum tree_code code, 
tree name,
  the forward-reference will be altered into a real type.  */
 
   ref = make_node (code);
-  if (flag_isoc23 && code != ENUMERAL_TYPE)
+  if (flag_isoc23 || code == ENUMERAL_TYPE)
 SET_TYPE_STRUCTURAL_EQUALITY (ref);
   if (code == ENUMERAL_TYPE)
 {
@@ -9919,6 +9919,7 @@ start_enum (location_t loc, struct c_enum_contents 
*the_enum, tree name,
 {
   enumtype = make_node (ENUMERAL_TYPE);
   TYPE_SIZE (enumtype) = NULL_TREE;
+  SET_TYPE_STRUCTURAL_EQUALITY (enumtype);
   pushtag (loc, name, enumtype);
   if (fixed_underlying_type != NULL_TREE)
{
@@ -9935,6 +9936,8 @@ start_enum (location_t loc, struct c_enum_contents 
*the_enum, tree name,
  TYPE_SIZE (enumtype) = NULL_TREE;
  TYPE_PRECISION (enumtype) = TYPE_PRECISION (fixed_underlying_type);
  ENUM_UNDERLYING_TYPE (enumtype) = fixed_underlying_type;
+ TYPE_CANONICAL (enumtype) = TYPE_CANONICAL (fixed_underlying_type);
+ c_update_type_canonical (enumtype);
  layout_type (enumtype);
}
 }
@@ -10094,6 +10097,10 @@ finish_enum (tree enumtype, tree values, tree 
attributes)
   ENUM_UNDERLYING_TYPE (enumtype) =
c_common_type_for_size (TYPE_PRECISION (tem), TYPE_UNSIGNED (tem));
 
+  TYPE_CANONICAL (enumtype) =
+   TYPE_CANONICAL (ENUM_UNDERLYING_TYPE (enumtype));
+  c_update_type_canonical (enumtype);
+
   layout_type (enumtype);
 }
 
diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc
index b7c72d2609c..55

[COMMITTED 26/31] ada: Fix strict aliasing violation in parameter passing

2024-05-21 Thread Marc Poulhiès
From: Eric Botcazou 

This fixes a long-standing (implicit) violation of the strict aliasing rules
that occurs when the result of a call to an instance of Unchecked_Conversion
is directly passed as an actual parameter in a call to a subprogram and the
passing mechanism is by reference.  In this case, the reference passed to
the subprogram may be to a type that has nothing to do with the type of the
underlying object, which is the definition of such a violation.

This implements the following two-pronged approach: first, the problematic
cases are detected and a reference to a temporary is passed instead of the
direct reference to the underlying object; second, the implementation of
pragma Universal_Aliasing is enhanced so that it is propagated from the
component type of an array type to the array type itself, or else can be
applied to the array type directly, and may therefore be used to prevent
the violation from occurring in the first place, when the array type is
involved in the Unchecked_Conversion.

gcc/ada/

* gcc-interface/decl.cc (gnat_to_gnu_entity) : Set
TYPE_TYPELESS_STORAGE on the array types if Universal_Aliasing is
set on the type or its component type.
: Likewise.
For other aggregate types, set TYPE_TYPELESS_STORAGE in this case.
(set_typeless_storage_on_aggregate_type): New function.
(set_universal_aliasing_on_type): Likewise.
* gcc-interface/trans.cc (Call_to_gnu): Add const to local variable.
Adjust comment.  Pass GNAT_NAME in the call to addressable_p and add
a bypass for atomic types in case it returns false.
(addressable_p): Add GNAT_EXPR third parameter with default value
and add a default value to the existing second parameter.
: Return false if the expression comes from a
function call and if the alias sets of source and target types are
both distinct from zero and each other.

Tested on x86_64-pc-linux-gnu, committed on master.

---
 gcc/ada/gcc-interface/decl.cc  | 40 ++-
 gcc/ada/gcc-interface/trans.cc | 60 --
 2 files changed, 82 insertions(+), 18 deletions(-)

diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
index 0987d534e69..ab54d2ccf13 100644
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -205,6 +205,8 @@ static Entity_Id Gigi_Cloned_Subtype (Entity_Id);
 static tree gnu_ext_name_for_subprog (Entity_Id, tree);
 static void set_nonaliased_component_on_array_type (tree);
 static void set_reverse_storage_order_on_array_type (tree);
+static void set_typeless_storage_on_aggregate_type (tree);
+static void set_universal_aliasing_on_type (tree);
 static bool same_discriminant_p (Entity_Id, Entity_Id);
 static bool array_type_has_nonaliased_component (tree, Entity_Id);
 static bool compile_time_known_address_p (Node_Id);
@@ -2385,6 +2387,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
  set_reverse_storage_order_on_array_type (tem);
if (array_type_has_nonaliased_component (tem, gnat_entity))
  set_nonaliased_component_on_array_type (tem);
+   if (Universal_Aliasing (gnat_entity)
+   || Universal_Aliasing (Component_Type (gnat_entity)))
+ set_typeless_storage_on_aggregate_type (tem);
  }
 
/* If this is a packed type implemented specially, then process the
@@ -2790,6 +2795,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
set_reverse_storage_order_on_array_type (gnu_type);
  if (array_type_has_nonaliased_component (gnu_type, gnat_entity))
set_nonaliased_component_on_array_type (gnu_type);
+ if (Universal_Aliasing (gnat_entity)
+ || Universal_Aliasing (Component_Type (gnat_entity)))
+   set_typeless_storage_on_aggregate_type (gnu_type);
 
  /* Clear the TREE_OVERFLOW flag, if any, for null arrays.  */
  if (gnu_null_ranges[index])
@@ -4757,7 +4765,17 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree 
gnu_expr, bool definition)
 
  /* Record whether a pragma Universal_Aliasing was specified.  */
  if (Universal_Aliasing (gnat_entity) && !TYPE_IS_DUMMY_P (gnu_type))
-   TYPE_UNIVERSAL_ALIASING_P (gnu_type) = 1;
+   {
+ /* Set TYPE_TYPELESS_STORAGE if this is an aggregate type and
+TYPE_UNIVERSAL_ALIASING_P otherwise, since the former is not
+available in the latter case  Both will effectively put alias
+set 0 on the type, but the former is more robust because it
+will be streamed in LTO mode.  */
+ if (AGGREGATE_TYPE_P (gnu_type))
+   set_typeless_storage_on_aggregate_type (gnu_type);
+ else
+   set_universal_aliasing_on_type (

[gcc r15-743] ada: Fix strict aliasing violation in parameter passing

2024-05-21 Thread Marc Poulhi?s via Gcc-cvs
https://gcc.gnu.org/g:f20a57edefe0cb185e57f82a15e887887b98d3ab

commit r15-743-gf20a57edefe0cb185e57f82a15e887887b98d3ab
Author: Eric Botcazou 
Date:   Thu Feb 29 09:14:27 2024 +0100

ada: Fix strict aliasing violation in parameter passing

This fixes a long-standing (implicit) violation of the strict aliasing rules
that occurs when the result of a call to an instance of Unchecked_Conversion
is directly passed as an actual parameter in a call to a subprogram and the
passing mechanism is by reference.  In this case, the reference passed to
the subprogram may be to a type that has nothing to do with the type of the
underlying object, which is the definition of such a violation.

This implements the following two-pronged approach: first, the problematic
cases are detected and a reference to a temporary is passed instead of the
direct reference to the underlying object; second, the implementation of
pragma Universal_Aliasing is enhanced so that it is propagated from the
component type of an array type to the array type itself, or else can be
applied to the array type directly, and may therefore be used to prevent
the violation from occurring in the first place, when the array type is
involved in the Unchecked_Conversion.

gcc/ada/

* gcc-interface/decl.cc (gnat_to_gnu_entity) : Set
TYPE_TYPELESS_STORAGE on the array types if Universal_Aliasing is
set on the type or its component type.
: Likewise.
For other aggregate types, set TYPE_TYPELESS_STORAGE in this case.
(set_typeless_storage_on_aggregate_type): New function.
(set_universal_aliasing_on_type): Likewise.
* gcc-interface/trans.cc (Call_to_gnu): Add const to local variable.
Adjust comment.  Pass GNAT_NAME in the call to addressable_p and add
a bypass for atomic types in case it returns false.
(addressable_p): Add GNAT_EXPR third parameter with default value
and add a default value to the existing second parameter.
: Return false if the expression comes from a
function call and if the alias sets of source and target types are
both distinct from zero and each other.

Diff:
---
 gcc/ada/gcc-interface/decl.cc  | 40 +++-
 gcc/ada/gcc-interface/trans.cc | 60 ++
 2 files changed, 82 insertions(+), 18 deletions(-)

diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
index 0987d534e69..ab54d2ccf13 100644
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -205,6 +205,8 @@ static Entity_Id Gigi_Cloned_Subtype (Entity_Id);
 static tree gnu_ext_name_for_subprog (Entity_Id, tree);
 static void set_nonaliased_component_on_array_type (tree);
 static void set_reverse_storage_order_on_array_type (tree);
+static void set_typeless_storage_on_aggregate_type (tree);
+static void set_universal_aliasing_on_type (tree);
 static bool same_discriminant_p (Entity_Id, Entity_Id);
 static bool array_type_has_nonaliased_component (tree, Entity_Id);
 static bool compile_time_known_address_p (Node_Id);
@@ -2385,6 +2387,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
  set_reverse_storage_order_on_array_type (tem);
if (array_type_has_nonaliased_component (tem, gnat_entity))
  set_nonaliased_component_on_array_type (tem);
+   if (Universal_Aliasing (gnat_entity)
+   || Universal_Aliasing (Component_Type (gnat_entity)))
+ set_typeless_storage_on_aggregate_type (tem);
  }
 
/* If this is a packed type implemented specially, then process the
@@ -2790,6 +2795,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
set_reverse_storage_order_on_array_type (gnu_type);
  if (array_type_has_nonaliased_component (gnu_type, gnat_entity))
set_nonaliased_component_on_array_type (gnu_type);
+ if (Universal_Aliasing (gnat_entity)
+ || Universal_Aliasing (Component_Type (gnat_entity)))
+   set_typeless_storage_on_aggregate_type (gnu_type);
 
  /* Clear the TREE_OVERFLOW flag, if any, for null arrays.  */
  if (gnu_null_ranges[index])
@@ -4757,7 +4765,17 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree 
gnu_expr, bool definition)
 
  /* Record whether a pragma Universal_Aliasing was specified.  */
  if (Universal_Aliasing (gnat_entity) && !TYPE_IS_DUMMY_P (gnu_type))
-   TYPE_UNIVERSAL_ALIASING_P (gnu_type) = 1;
+   {
+ /* Set TYPE_TYPELESS_STORAGE if this is an aggregate type and
+TYPE_UNIVERSAL_ALIASING_P otherwise, since the former is not
+available in the latter case  Both will effectively p

[COMMITTED 28/31] ada: Fix strict aliasing violation in parameter passing (continued)

2024-05-21 Thread Marc Poulhiès
From: Eric Botcazou 

This fixes another long-standing (implicit) violation of the strict aliasing
rules that occurs when the result of a value conversion is directly passed
as an actual parameter in a call to a subprogram and the passing mechanism
is by reference.  In this case, the reference passed to the subprogram may
be to a type that is too different from the type of the underlying object,
which is the definition of such a violation.

The change reworks and strengthens the previous fix as follows: first, the
detection of these violations is moved into a dedicated predicate; second,
an assertion is added to check that none of them has been missed, which is
triggered by either -fchecking or -fstrict-aliasing, as the closely related
assertion that is present in relate_alias_sets.

The assertion uncovered two internal sources of violations: implementation
types for packed array types with peculiar index types and interface types,
which are fixed by propagating alias sets in the first case and resorting to
universal aliasing in the second case.

Finally, an unconditional warning is implemented to inform the user that the
temporary is created and to suggest a possible solution to prevent that.

gcc/ada/

* gcc-interface/decl.cc (gnat_to_gnu_entity) : For a
packed type implemented specially, temporarily save the XUA type as
equivalent to the entity before processing the implementation type.
For this implementation type, if its component type is the same as
that of the original type, copy the alias set from the latter.
: Resort to universal aliasing for all interface types.
* gcc-interface/trans.cc (Call_to_gnu): Add GNU_ACTUAL_TYPE local
variable and rename existing one to GNU_UNPADDED_ACTUAL_TYPE.
If the formal is passed by reference and the actual is a conversion,
call aliasable_p to detect aliasing violations, issue a warning upon
finding one and create the temporary in the target type.
Add an assertion that no such violation has been missed above.
(addressable_p): Revert latest changes.
(aliasable_p): New predicate.
* gcc-interface/utils2.cc (build_binary_op) : When
creating a new array type on the fly, preserve the alias set of the
operation type.

Tested on x86_64-pc-linux-gnu, committed on master.

---
 gcc/ada/gcc-interface/decl.cc   |  48 ++---
 gcc/ada/gcc-interface/trans.cc  | 167 +++-
 gcc/ada/gcc-interface/utils2.cc |   6 +-
 3 files changed, 159 insertions(+), 62 deletions(-)

diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
index ab54d2ccf13..6e40a157734 100644
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -2119,6 +2119,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
 
 case E_Array_Type:
   {
+   const Entity_Id OAT = Original_Array_Type (gnat_entity);
const Entity_Id PAT = Packed_Array_Impl_Type (gnat_entity);
const bool convention_fortran_p
  = (Convention (gnat_entity) == Convention_Fortran);
@@ -2392,14 +2393,10 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree 
gnu_expr, bool definition)
  set_typeless_storage_on_aggregate_type (tem);
  }
 
-   /* If this is a packed type implemented specially, then process the
-  implementation type so it is elaborated in the proper scope.  */
-   if (Present (PAT))
- gnat_to_gnu_entity (PAT, NULL_TREE, false);
-
-   /* Otherwise, if an alignment is specified, use it if valid and, if
-  the alignment was requested with an explicit clause, state so.  */
-   else if (Known_Alignment (gnat_entity))
+   /* If an alignment is specified for an array that is not a packed type
+  implemented specially, use the alignment if it is valid and, if it
+  was requested with an explicit clause, preserve the information.  */
+   if (Known_Alignment (gnat_entity) && No (PAT))
  {
SET_TYPE_ALIGN (tem,
validate_alignment (Alignment (gnat_entity),
@@ -2418,7 +2415,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
 
TYPE_BIT_PACKED_ARRAY_TYPE_P (tem)
  = (Is_Packed_Array_Impl_Type (gnat_entity)
-? Is_Bit_Packed_Array (Original_Array_Type (gnat_entity))
+? Is_Bit_Packed_Array (OAT)
 : Is_Bit_Packed_Array (gnat_entity));
 
if (Treat_As_Volatile (gnat_entity))
@@ -2447,8 +2444,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
  TYPE_ARRAY_MAX_SIZE (tem) = gnu_max_size;
 
/* See the above description for the rationale.  */
-   create_type_decl (create_concat_name (gnat_entity, "XUA"), tem,
- artificial_p, debug_info_p, gnat_entity);
+   tree gnu_tmp_decl
+ 

[gcc r15-745] ada: Fix strict aliasing violation in parameter passing (continued)

2024-05-21 Thread Marc Poulhi?s via Gcc-cvs
https://gcc.gnu.org/g:ccdef2aef6463159a596ef1a4afe6ddce9d0f2ea

commit r15-745-gccdef2aef6463159a596ef1a4afe6ddce9d0f2ea
Author: Eric Botcazou 
Date:   Sun Mar 10 13:22:55 2024 +0100

ada: Fix strict aliasing violation in parameter passing (continued)

This fixes another long-standing (implicit) violation of the strict aliasing
rules that occurs when the result of a value conversion is directly passed
as an actual parameter in a call to a subprogram and the passing mechanism
is by reference.  In this case, the reference passed to the subprogram may
be to a type that is too different from the type of the underlying object,
which is the definition of such a violation.

The change reworks and strengthens the previous fix as follows: first, the
detection of these violations is moved into a dedicated predicate; second,
an assertion is added to check that none of them has been missed, which is
triggered by either -fchecking or -fstrict-aliasing, as the closely related
assertion that is present in relate_alias_sets.

The assertion uncovered two internal sources of violations: implementation
types for packed array types with peculiar index types and interface types,
which are fixed by propagating alias sets in the first case and resorting to
universal aliasing in the second case.

Finally, an unconditional warning is implemented to inform the user that the
temporary is created and to suggest a possible solution to prevent that.

gcc/ada/

* gcc-interface/decl.cc (gnat_to_gnu_entity) : For a
packed type implemented specially, temporarily save the XUA type as
equivalent to the entity before processing the implementation type.
For this implementation type, if its component type is the same as
that of the original type, copy the alias set from the latter.
: Resort to universal aliasing for all interface types.
* gcc-interface/trans.cc (Call_to_gnu): Add GNU_ACTUAL_TYPE local
variable and rename existing one to GNU_UNPADDED_ACTUAL_TYPE.
If the formal is passed by reference and the actual is a conversion,
call aliasable_p to detect aliasing violations, issue a warning upon
finding one and create the temporary in the target type.
Add an assertion that no such violation has been missed above.
(addressable_p): Revert latest changes.
(aliasable_p): New predicate.
* gcc-interface/utils2.cc (build_binary_op) : When
creating a new array type on the fly, preserve the alias set of the
operation type.

Diff:
---
 gcc/ada/gcc-interface/decl.cc   |  48 
 gcc/ada/gcc-interface/trans.cc  | 167 +---
 gcc/ada/gcc-interface/utils2.cc |   6 +-
 3 files changed, 159 insertions(+), 62 deletions(-)

diff --git a/gcc/ada/gcc-interface/decl.cc b/gcc/ada/gcc-interface/decl.cc
index ab54d2ccf13..6e40a157734 100644
--- a/gcc/ada/gcc-interface/decl.cc
+++ b/gcc/ada/gcc-interface/decl.cc
@@ -2119,6 +2119,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
 
 case E_Array_Type:
   {
+   const Entity_Id OAT = Original_Array_Type (gnat_entity);
const Entity_Id PAT = Packed_Array_Impl_Type (gnat_entity);
const bool convention_fortran_p
  = (Convention (gnat_entity) == Convention_Fortran);
@@ -2392,14 +2393,10 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree 
gnu_expr, bool definition)
  set_typeless_storage_on_aggregate_type (tem);
  }
 
-   /* If this is a packed type implemented specially, then process the
-  implementation type so it is elaborated in the proper scope.  */
-   if (Present (PAT))
- gnat_to_gnu_entity (PAT, NULL_TREE, false);
-
-   /* Otherwise, if an alignment is specified, use it if valid and, if
-  the alignment was requested with an explicit clause, state so.  */
-   else if (Known_Alignment (gnat_entity))
+   /* If an alignment is specified for an array that is not a packed type
+  implemented specially, use the alignment if it is valid and, if it
+  was requested with an explicit clause, preserve the information.  */
+   if (Known_Alignment (gnat_entity) && No (PAT))
  {
SET_TYPE_ALIGN (tem,
validate_alignment (Alignment (gnat_entity),
@@ -2418,7 +2415,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, 
bool definition)
 
TYPE_BIT_PACKED_ARRAY_TYPE_P (tem)
  = (Is_Packed_Array_Impl_Type (gnat_entity)
-? Is_Bit_Packed_Array (Original_Array_Type (gnat_entity))
+? Is_Bit_Packed_Array (OAT)
 : Is_Bit_Packed_Array (gnat_entity));
 
if (Treat_As_Volatile (gnat_entity))
@@ -2447,8 +2444,9 @@ gnat_to_gnu_entity (E

[COMMITTED 01/30] ada: Rework and augment documentation on strict aliasing

2024-05-20 Thread Marc Poulhiès
From: Eric Botcazou 

The documentation was originally centered around pragma No_Strict_Aliasing
and pragma Universal_Aliasing was mentioned only as an afterthought.  It
also contained a warning about the usage of overlays implemented by means
of address clauses that has been obsolete for long.

gcc/ada/

* doc/gnat_rm/implementation_defined_pragmas.rst
(Universal_Aliasing): Remove reference to No_Strict_Aliasing.
* doc/gnat_ugn/gnat_and_program_execution.rst
(Optimization and Strict Aliasinng): Simplify first example and
make it more consistent with the second.  Add description of the
effects of pragma Universal_Aliasing and document new warning
issued for unchecked conversions.  Remove obsolete stuff.
* gnat_rm.texi: Regenerate.
* gnat_ugn.texi: Regenerate.

Tested on x86_64-pc-linux-gnu, committed on master.

---
 .../implementation_defined_pragmas.rst|   7 +-
 .../gnat_ugn/gnat_and_program_execution.rst   | 296 +
 gcc/ada/gnat_rm.texi  |   7 +-
 gcc/ada/gnat_ugn.texi | 306 ++
 4 files changed, 353 insertions(+), 263 deletions(-)

diff --git a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst 
b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst
index 7f221e32344..bcbd85984dc 100644
--- a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst
+++ b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst
@@ -6949,10 +6949,9 @@ Syntax:
 
 ``type_LOCAL_NAME`` must refer to a type declaration in the current
 declarative part.  The effect is to inhibit strict type-based aliasing
-optimization for the given type.  In other words, the effect is as though
-access types designating this type were subject to pragma No_Strict_Aliasing.
-For a detailed description of the strict aliasing optimization, and the
-situations in which it must be suppressed, see the section on
+optimizations for the given type.  For a detailed description of the
+strict type-based aliasing optimizations and the situations in which
+they need to be suppressed, see the section on
 ``Optimization and Strict Aliasing`` in the :title:`GNAT User's Guide`.
 
 .. _Pragma-Unmodified:
diff --git a/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst 
b/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst
index 35e34772658..d502da87eb0 100644
--- a/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst
+++ b/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst
@@ -2072,37 +2072,36 @@ the following example:
 
   .. code-block:: ada
 
- procedure R is
+ procedure M is
 type Int1 is new Integer;
+I1 : Int1;
+
 type Int2 is new Integer;
-type Int1A is access Int1;
-type Int2A is access Int2;
-Int1V : Int1A;
-Int2V : Int2A;
+type A2 is access Int2;
+V2 : A2;
 ...
 
  begin
 ...
 for J in Data'Range loop
-   if Data (J) = Int1V.all then
-  Int2V.all := Int2V.all + 1;
+   if Data (J) = I1 then
+  V2.all := V2.all + 1;
end if;
 end loop;
 ...
- end R;
+ end;
 
-In this example, since the variable ``Int1V`` can only access objects
-of type ``Int1``, and ``Int2V`` can only access objects of type
-``Int2``, there is no possibility that the assignment to
-``Int2V.all`` affects the value of ``Int1V.all``. This means that
-the compiler optimizer can "know" that the value ``Int1V.all`` is constant
-for all iterations of the loop and avoid the extra memory reference
-required to dereference it each time through the loop.
+In this example, since ``V2`` can only access objects of type ``Int2``
+and ``I1`` is not one of them, there is no possibility that the assignment
+to ``V2.all`` affects the value of ``I1``. This means that the compiler
+optimizer can infer that the value ``I1`` is constant for all iterations
+of the loop and load it from memory only once, before entering the loop,
+instead of in every iteration (this is called load hoisting).
 
-This kind of optimization, called strict aliasing analysis, is
+This kind of optimizations, based on strict type-based aliasing, is
 triggered by specifying an optimization level of :switch:`-O2` or
-higher or :switch:`-Os` and allows GNAT to generate more efficient code
-when access values are involved.
+higher (or :switch:`-Os`) and allows the compiler to generate more
+efficient code.
 
 However, although this optimization is always correct in terms of
 the formal semantics of the Ada Reference Manual, difficulties can
@@ -2111,173 +2110,214 @@ the typing system. Consider the following complete 
program example:
 
   .. code-block:: ada
 
-  package p1 is
- type int1 is new integer;
- type int2 is new integer;
- type a1 is access int1;
- type a2 is access int2;
-  end p1;
+  package P1 is
+ type Int1 is n

[COMMITTED 05/30] ada: One more adjustment coming from aliasing considerations

2024-05-20 Thread Marc Poulhiès
From: Eric Botcazou 

It is needed on PowerPC platforms because of specific calling conventions.

gcc/ada/

* libgnat/g-sothco.ads (In_Addr): Add aspect Universal_Aliasing.

Tested on x86_64-pc-linux-gnu, committed on master.

---
 gcc/ada/libgnat/g-sothco.ads | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/ada/libgnat/g-sothco.ads b/gcc/ada/libgnat/g-sothco.ads
index 8c219333649..da1e6f5bcdd 100644
--- a/gcc/ada/libgnat/g-sothco.ads
+++ b/gcc/ada/libgnat/g-sothco.ads
@@ -123,10 +123,13 @@ package GNAT.Sockets.Thin_Common is
 
type In_Addr is record
   S_B1, S_B2, S_B3, S_B4 : C.unsigned_char;
-   end record with Convention => C, Alignment => C.int'Alignment;
+   end record
+ with Convention => C, Alignment  => C.int'Alignment, Universal_Aliasing;
--  IPv4 address, represented as a network-order C.int. Note that the
--  underlying operating system may assume that values of this type have
-   --  C.int alignment, so we need to provide a suitable alignment clause here.
+   --  C.int's alignment, so we need to provide a suitable alignment clause.
+   --  We also need to inhibit strict type-based aliasing optimizations in
+   --  order to implement the following unchecked conversions efficiently.
 
function To_In_Addr is new Ada.Unchecked_Conversion (C.int, In_Addr);
function To_Int is new Ada.Unchecked_Conversion (In_Addr, C.int);
-- 
2.43.2



[gcc r15-664] ada: One more adjustment coming from aliasing considerations

2024-05-20 Thread Marc Poulhi?s via Gcc-cvs
https://gcc.gnu.org/g:eef3025547ce55cbf6a9018b495ef5c9a562047a

commit r15-664-geef3025547ce55cbf6a9018b495ef5c9a562047a
Author: Eric Botcazou 
Date:   Tue Mar 19 10:56:34 2024 +0100

ada: One more adjustment coming from aliasing considerations

It is needed on PowerPC platforms because of specific calling conventions.

gcc/ada/

* libgnat/g-sothco.ads (In_Addr): Add aspect Universal_Aliasing.

Diff:
---
 gcc/ada/libgnat/g-sothco.ads | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/ada/libgnat/g-sothco.ads b/gcc/ada/libgnat/g-sothco.ads
index 8c2193336491..da1e6f5bcddf 100644
--- a/gcc/ada/libgnat/g-sothco.ads
+++ b/gcc/ada/libgnat/g-sothco.ads
@@ -123,10 +123,13 @@ package GNAT.Sockets.Thin_Common is
 
type In_Addr is record
   S_B1, S_B2, S_B3, S_B4 : C.unsigned_char;
-   end record with Convention => C, Alignment => C.int'Alignment;
+   end record
+ with Convention => C, Alignment  => C.int'Alignment, Universal_Aliasing;
--  IPv4 address, represented as a network-order C.int. Note that the
--  underlying operating system may assume that values of this type have
-   --  C.int alignment, so we need to provide a suitable alignment clause here.
+   --  C.int's alignment, so we need to provide a suitable alignment clause.
+   --  We also need to inhibit strict type-based aliasing optimizations in
+   --  order to implement the following unchecked conversions efficiently.
 
function To_In_Addr is new Ada.Unchecked_Conversion (C.int, In_Addr);
function To_Int is new Ada.Unchecked_Conversion (In_Addr, C.int);


[gcc r15-660] ada: Rework and augment documentation on strict aliasing

2024-05-20 Thread Marc Poulhi?s via Gcc-cvs
https://gcc.gnu.org/g:5d6c099ffaa384425f33d4e3a52f55149b9bc99a

commit r15-660-g5d6c099ffaa384425f33d4e3a52f55149b9bc99a
Author: Eric Botcazou 
Date:   Wed Mar 13 17:05:12 2024 +0100

ada: Rework and augment documentation on strict aliasing

The documentation was originally centered around pragma No_Strict_Aliasing
and pragma Universal_Aliasing was mentioned only as an afterthought.  It
also contained a warning about the usage of overlays implemented by means
of address clauses that has been obsolete for long.

gcc/ada/

* doc/gnat_rm/implementation_defined_pragmas.rst
(Universal_Aliasing): Remove reference to No_Strict_Aliasing.
* doc/gnat_ugn/gnat_and_program_execution.rst
(Optimization and Strict Aliasinng): Simplify first example and
make it more consistent with the second.  Add description of the
effects of pragma Universal_Aliasing and document new warning
issued for unchecked conversions.  Remove obsolete stuff.
* gnat_rm.texi: Regenerate.
* gnat_ugn.texi: Regenerate.

Diff:
---
 .../doc/gnat_rm/implementation_defined_pragmas.rst |   7 +-
 .../doc/gnat_ugn/gnat_and_program_execution.rst| 296 +++-
 gcc/ada/gnat_rm.texi   |   7 +-
 gcc/ada/gnat_ugn.texi  | 306 -
 4 files changed, 353 insertions(+), 263 deletions(-)

diff --git a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst 
b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst
index 7f221e323446..bcbd85984dc0 100644
--- a/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst
+++ b/gcc/ada/doc/gnat_rm/implementation_defined_pragmas.rst
@@ -6949,10 +6949,9 @@ Syntax:
 
 ``type_LOCAL_NAME`` must refer to a type declaration in the current
 declarative part.  The effect is to inhibit strict type-based aliasing
-optimization for the given type.  In other words, the effect is as though
-access types designating this type were subject to pragma No_Strict_Aliasing.
-For a detailed description of the strict aliasing optimization, and the
-situations in which it must be suppressed, see the section on
+optimizations for the given type.  For a detailed description of the
+strict type-based aliasing optimizations and the situations in which
+they need to be suppressed, see the section on
 ``Optimization and Strict Aliasing`` in the :title:`GNAT User's Guide`.
 
 .. _Pragma-Unmodified:
diff --git a/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst 
b/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst
index 35e347726589..d502da87eb0b 100644
--- a/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst
+++ b/gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst
@@ -2072,37 +2072,36 @@ the following example:
 
   .. code-block:: ada
 
- procedure R is
+ procedure M is
 type Int1 is new Integer;
+I1 : Int1;
+
 type Int2 is new Integer;
-type Int1A is access Int1;
-type Int2A is access Int2;
-Int1V : Int1A;
-Int2V : Int2A;
+type A2 is access Int2;
+V2 : A2;
 ...
 
  begin
 ...
 for J in Data'Range loop
-   if Data (J) = Int1V.all then
-  Int2V.all := Int2V.all + 1;
+   if Data (J) = I1 then
+  V2.all := V2.all + 1;
end if;
 end loop;
 ...
- end R;
+ end;
 
-In this example, since the variable ``Int1V`` can only access objects
-of type ``Int1``, and ``Int2V`` can only access objects of type
-``Int2``, there is no possibility that the assignment to
-``Int2V.all`` affects the value of ``Int1V.all``. This means that
-the compiler optimizer can "know" that the value ``Int1V.all`` is constant
-for all iterations of the loop and avoid the extra memory reference
-required to dereference it each time through the loop.
+In this example, since ``V2`` can only access objects of type ``Int2``
+and ``I1`` is not one of them, there is no possibility that the assignment
+to ``V2.all`` affects the value of ``I1``. This means that the compiler
+optimizer can infer that the value ``I1`` is constant for all iterations
+of the loop and load it from memory only once, before entering the loop,
+instead of in every iteration (this is called load hoisting).
 
-This kind of optimization, called strict aliasing analysis, is
+This kind of optimizations, based on strict type-based aliasing, is
 triggered by specifying an optimization level of :switch:`-O2` or
-higher or :switch:`-Os` and allows GNAT to generate more efficient code
-when access values are involved.
+higher (or :switch:`-Os`) and allows the compiler to generate more
+efficient code.
 
 However, although this optimization is always correct in terms of
 the formal semantics of the Ada Reference Manual, difficulties can
@@ -2111,173 +2110,214 @@ the typing system. Consider the following complet

[COMMITTED 30/35] ada: Further adjustments coming from aliasing considerations

2024-05-17 Thread Marc Poulhiès
From: Eric Botcazou 

They are needed on 32-bit platforms because of different calling conventions
and again in the units implementing AltiVec and Streams support.

gcc/ada/

* libgnat/g-alvevi.ads: Add pragma Universal_Aliasing for all the
view types.
* libgnat/s-stratt.ads: Likewise for Fat_Pointer type.

Tested on x86_64-pc-linux-gnu, committed on master.

---
 gcc/ada/libgnat/g-alvevi.ads | 11 +++
 gcc/ada/libgnat/s-stratt.ads |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/gcc/ada/libgnat/g-alvevi.ads b/gcc/ada/libgnat/g-alvevi.ads
index b2beac7284c..b0f58790adf 100644
--- a/gcc/ada/libgnat/g-alvevi.ads
+++ b/gcc/ada/libgnat/g-alvevi.ads
@@ -58,6 +58,7 @@ package GNAT.Altivec.Vector_Views is
type VUC_View is record
   Values : Varray_unsigned_char;
end record;
+   pragma Universal_Aliasing (VUC_View);
 
type Varray_signed_char is array (Vchar_Range) of signed_char;
for Varray_signed_char'Alignment use VECTOR_ALIGNMENT;
@@ -65,6 +66,7 @@ package GNAT.Altivec.Vector_Views is
type VSC_View is record
   Values : Varray_signed_char;
end record;
+   pragma Universal_Aliasing (VSC_View);
 
type Varray_bool_char is array (Vchar_Range) of bool_char;
for Varray_bool_char'Alignment use VECTOR_ALIGNMENT;
@@ -72,6 +74,7 @@ package GNAT.Altivec.Vector_Views is
type VBC_View is record
   Values : Varray_bool_char;
end record;
+   pragma Universal_Aliasing (VBC_View);
 
--
-- short components --
@@ -85,6 +88,7 @@ package GNAT.Altivec.Vector_Views is
type VUS_View is record
   Values : Varray_unsigned_short;
end record;
+   pragma Universal_Aliasing (VUS_View);
 
type Varray_signed_short is array (Vshort_Range) of signed_short;
for Varray_signed_short'Alignment use VECTOR_ALIGNMENT;
@@ -92,6 +96,7 @@ package GNAT.Altivec.Vector_Views is
type VSS_View is record
   Values : Varray_signed_short;
end record;
+   pragma Universal_Aliasing (VSS_View);
 
type Varray_bool_short is array (Vshort_Range) of bool_short;
for Varray_bool_short'Alignment use VECTOR_ALIGNMENT;
@@ -99,6 +104,7 @@ package GNAT.Altivec.Vector_Views is
type VBS_View is record
   Values : Varray_bool_short;
end record;
+   pragma Universal_Aliasing (VBS_View);
 

-- int components --
@@ -112,6 +118,7 @@ package GNAT.Altivec.Vector_Views is
type VUI_View is record
   Values : Varray_unsigned_int;
end record;
+   pragma Universal_Aliasing (VUI_View);
 
type Varray_signed_int is array (Vint_Range) of signed_int;
for Varray_signed_int'Alignment use VECTOR_ALIGNMENT;
@@ -119,6 +126,7 @@ package GNAT.Altivec.Vector_Views is
type VSI_View is record
   Values : Varray_signed_int;
end record;
+   pragma Universal_Aliasing (VSI_View);
 
type Varray_bool_int is array (Vint_Range) of bool_int;
for Varray_bool_int'Alignment use VECTOR_ALIGNMENT;
@@ -126,6 +134,7 @@ package GNAT.Altivec.Vector_Views is
type VBI_View is record
   Values : Varray_bool_int;
end record;
+   pragma Universal_Aliasing (VBI_View);
 
--
-- float components --
@@ -139,6 +148,7 @@ package GNAT.Altivec.Vector_Views is
type VF_View is record
   Values : Varray_float;
end record;
+   pragma Universal_Aliasing (VF_View);
 
--
-- pixel components --
@@ -152,5 +162,6 @@ package GNAT.Altivec.Vector_Views is
type VP_View is record
   Values : Varray_pixel;
end record;
+   pragma Universal_Aliasing (VP_View);
 
 end GNAT.Altivec.Vector_Views;
diff --git a/gcc/ada/libgnat/s-stratt.ads b/gcc/ada/libgnat/s-stratt.ads
index 1d4c82d17ab..eee19f4bdce 100644
--- a/gcc/ada/libgnat/s-stratt.ads
+++ b/gcc/ada/libgnat/s-stratt.ads
@@ -74,6 +74,9 @@ package System.Stream_Attributes is
   P2 : System.Address;
end record;
 
+   pragma Universal_Aliasing (Fat_Pointer);
+   --  This avoids a copy for the aforementioned unchecked conversions
+

-- Treatment of enumeration types --

-- 
2.43.2



[COMMITTED 19/35] ada: Couple of adjustments coming from aliasing considerations

2024-05-17 Thread Marc Poulhiès
From: Eric Botcazou 

The first adjustment is to the expansion of implementation types for array
types with peculiar index types, for which the aliased property set on the
component of the original type must be copied; the implicit base type also
needs to be properly marked if the implementation type is constrained.

The second adjustment is to selected types in the runtime, which need to
be marked with pragma Universal_Aliasing because of their special usage.

gcc/ada/

* exp_pakd.adb (Create_Packed_Array_Impl_Type): For non-bit-packed
array types, propagate the aliased property of the component.
(Install_PAT): Set fields on the implicit base type of an array.
* libgnat/a-stream.ads (private part): Add pragma Universal_Aliasing
for Stream_Element.
* libgnat/g-alleve.ads: Add pragma Universal_Aliasing for all the
vector types.
* libgnat/g-alleve__hard.ads: Likewise.

Tested on x86_64-pc-linux-gnu, committed on master.

---
 gcc/ada/exp_pakd.adb   | 12 +--
 gcc/ada/libgnat/a-stream.ads   |  3 ++
 gcc/ada/libgnat/g-alleve.ads   | 54 ++
 gcc/ada/libgnat/g-alleve__hard.ads | 11 ++
 4 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/gcc/ada/exp_pakd.adb b/gcc/ada/exp_pakd.adb
index 3f26c3527fa..59dfe5df8df 100644
--- a/gcc/ada/exp_pakd.adb
+++ b/gcc/ada/exp_pakd.adb
@@ -598,6 +598,14 @@ package body Exp_Pakd is
  Set_Associated_Node_For_Itype (PAT, Typ);
  Set_Original_Array_Type   (PAT, Typ);
 
+ --  In the case of a constrained array type, also set fields on the
+ --  implicit base type built during the analysis of its declaration.
+
+ if Ekind (PAT) = E_Array_Subtype then
+Set_Is_Packed_Array_Impl_Type (Etype (PAT), True);
+Set_Original_Array_Type   (Etype (PAT), Base_Type (Typ));
+ end if;
+
  --  Propagate representation aspects
 
  Set_Is_Atomic   (PAT, Is_Atomic(Typ));
@@ -818,7 +826,7 @@ package body Exp_Pakd is
Subtype_Marks => Indexes,
Component_Definition =>
  Make_Component_Definition (Loc,
-   Aliased_Present=> False,
+   Aliased_Present=> Has_Aliased_Components (Typ),
Subtype_Indication =>
   New_Occurrence_Of (Ctyp, Loc)));
 
@@ -828,7 +836,7 @@ package body Exp_Pakd is
 Discrete_Subtype_Definitions => Indexes,
 Component_Definition =>
   Make_Component_Definition (Loc,
-Aliased_Present=> False,
+Aliased_Present=> Has_Aliased_Components (Typ),
 Subtype_Indication =>
   New_Occurrence_Of (Ctyp, Loc)));
 end if;
diff --git a/gcc/ada/libgnat/a-stream.ads b/gcc/ada/libgnat/a-stream.ads
index 0a0cabce3f2..dcb5a9aa81c 100644
--- a/gcc/ada/libgnat/a-stream.ads
+++ b/gcc/ada/libgnat/a-stream.ads
@@ -84,4 +84,7 @@ private
for Stream_Element_Array'Read use Read_SEA;
for Stream_Element_Array'Write use Write_SEA;
 
+   pragma Universal_Aliasing (Stream_Element);
+   --  This type is used to stream any other type
+
 end Ada.Streams;
diff --git a/gcc/ada/libgnat/g-alleve.ads b/gcc/ada/libgnat/g-alleve.ads
index 0f3ec36d0f1..4e22a3e6387 100644
--- a/gcc/ada/libgnat/g-alleve.ads
+++ b/gcc/ada/libgnat/g-alleve.ads
@@ -313,22 +313,62 @@ private
---
 
--  We simply use the natural array definitions corresponding to each
-   --  user-level vector type.
+   --  user-level vector type. We need to put pragma Universal_Aliasing
+   --  on these types because the common operations are implemented by
+   --  means of Unchecked_Conversion betwwen different representations.
 
-   type LL_VUI is new VUI_View;
-   type LL_VSI is new VSI_View;
-   type LL_VBI is new VBI_View;
+   --
+   -- char Core Components --
+   --
+
+   type LL_VUC is new VUC_View;
+   pragma Universal_Aliasing (LL_VUC);
+
+   type LL_VSC is new VSC_View;
+   pragma Universal_Aliasing (LL_VSC);
+
+   type LL_VBC is new VBC_View;
+   pragma Universal_Aliasing (LL_VBC);
+
+   ---
+   -- short Core Components --
+   ---
 
type LL_VUS is new VUS_View;
+   pragma Universal_Aliasing (LL_VUS);
+
type LL_VSS is new VSS_View;
+   pragma Universal_Aliasing (LL_VSS);
+
type LL_VBS is new VBS_View;
+   pragma Universal_Aliasing (LL_VBS);
 
-   type LL_VUC is new VUC_View;
-   type LL_VSC is new VSC_View;
-   type LL_VBC is new VBC_View;
+   -
+   -- int Core Components --
+   -
+
+   type LL_VUI is new VUI_View;
+   pragma Universal_Aliasing (LL_VUI);
+
+   type LL_VSI is new 

[gcc r15-605] ada: Couple of adjustments coming from aliasing considerations

2024-05-17 Thread Marc Poulhi?s via Gcc-cvs
https://gcc.gnu.org/g:485d595d22c7800eb214034c9b58211ab232dbbf

commit r15-605-g485d595d22c7800eb214034c9b58211ab232dbbf
Author: Eric Botcazou 
Date:   Sun Mar 10 15:51:21 2024 +0100

ada: Couple of adjustments coming from aliasing considerations

The first adjustment is to the expansion of implementation types for array
types with peculiar index types, for which the aliased property set on the
component of the original type must be copied; the implicit base type also
needs to be properly marked if the implementation type is constrained.

The second adjustment is to selected types in the runtime, which need to
be marked with pragma Universal_Aliasing because of their special usage.

gcc/ada/

* exp_pakd.adb (Create_Packed_Array_Impl_Type): For non-bit-packed
array types, propagate the aliased property of the component.
(Install_PAT): Set fields on the implicit base type of an array.
* libgnat/a-stream.ads (private part): Add pragma Universal_Aliasing
for Stream_Element.
* libgnat/g-alleve.ads: Add pragma Universal_Aliasing for all the
vector types.
* libgnat/g-alleve__hard.ads: Likewise.

Diff:
---
 gcc/ada/exp_pakd.adb   | 12 +++--
 gcc/ada/libgnat/a-stream.ads   |  3 +++
 gcc/ada/libgnat/g-alleve.ads   | 54 +-
 gcc/ada/libgnat/g-alleve__hard.ads | 11 
 4 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/gcc/ada/exp_pakd.adb b/gcc/ada/exp_pakd.adb
index 3f26c3527fa4..59dfe5df8df4 100644
--- a/gcc/ada/exp_pakd.adb
+++ b/gcc/ada/exp_pakd.adb
@@ -598,6 +598,14 @@ package body Exp_Pakd is
  Set_Associated_Node_For_Itype (PAT, Typ);
  Set_Original_Array_Type   (PAT, Typ);
 
+ --  In the case of a constrained array type, also set fields on the
+ --  implicit base type built during the analysis of its declaration.
+
+ if Ekind (PAT) = E_Array_Subtype then
+Set_Is_Packed_Array_Impl_Type (Etype (PAT), True);
+Set_Original_Array_Type   (Etype (PAT), Base_Type (Typ));
+ end if;
+
  --  Propagate representation aspects
 
  Set_Is_Atomic   (PAT, Is_Atomic(Typ));
@@ -818,7 +826,7 @@ package body Exp_Pakd is
Subtype_Marks => Indexes,
Component_Definition =>
  Make_Component_Definition (Loc,
-   Aliased_Present=> False,
+   Aliased_Present=> Has_Aliased_Components (Typ),
Subtype_Indication =>
   New_Occurrence_Of (Ctyp, Loc)));
 
@@ -828,7 +836,7 @@ package body Exp_Pakd is
 Discrete_Subtype_Definitions => Indexes,
 Component_Definition =>
   Make_Component_Definition (Loc,
-Aliased_Present=> False,
+Aliased_Present=> Has_Aliased_Components (Typ),
 Subtype_Indication =>
   New_Occurrence_Of (Ctyp, Loc)));
 end if;
diff --git a/gcc/ada/libgnat/a-stream.ads b/gcc/ada/libgnat/a-stream.ads
index 0a0cabce3f2b..dcb5a9aa81ce 100644
--- a/gcc/ada/libgnat/a-stream.ads
+++ b/gcc/ada/libgnat/a-stream.ads
@@ -84,4 +84,7 @@ private
for Stream_Element_Array'Read use Read_SEA;
for Stream_Element_Array'Write use Write_SEA;
 
+   pragma Universal_Aliasing (Stream_Element);
+   --  This type is used to stream any other type
+
 end Ada.Streams;
diff --git a/gcc/ada/libgnat/g-alleve.ads b/gcc/ada/libgnat/g-alleve.ads
index 0f3ec36d0f1b..4e22a3e63876 100644
--- a/gcc/ada/libgnat/g-alleve.ads
+++ b/gcc/ada/libgnat/g-alleve.ads
@@ -313,22 +313,62 @@ private
---
 
--  We simply use the natural array definitions corresponding to each
-   --  user-level vector type.
+   --  user-level vector type. We need to put pragma Universal_Aliasing
+   --  on these types because the common operations are implemented by
+   --  means of Unchecked_Conversion betwwen different representations.
 
-   type LL_VUI is new VUI_View;
-   type LL_VSI is new VSI_View;
-   type LL_VBI is new VBI_View;
+   --
+   -- char Core Components --
+   --
+
+   type LL_VUC is new VUC_View;
+   pragma Universal_Aliasing (LL_VUC);
+
+   type LL_VSC is new VSC_View;
+   pragma Universal_Aliasing (LL_VSC);
+
+   type LL_VBC is new VBC_View;
+   pragma Universal_Aliasing (LL_VBC);
+
+   ---
+   -- short Core Components --
+   ---
 
type LL_VUS is new VUS_View;
+   pragma Universal_Aliasing (LL_VUS);
+
type LL_VSS is new VSS_View;
+   pragma Universal_Aliasing (LL_VSS);
+
type LL_VBS is new VBS_View;
+   pragma Universal_Aliasi

[gcc r15-616] ada: Further adjustments coming from aliasing considerations

2024-05-17 Thread Marc Poulhi?s via Gcc-cvs
https://gcc.gnu.org/g:9ff83f013eb1ea2eac11c17cc3be2024e96101a5

commit r15-616-g9ff83f013eb1ea2eac11c17cc3be2024e96101a5
Author: Eric Botcazou 
Date:   Thu Mar 14 12:58:29 2024 +0100

ada: Further adjustments coming from aliasing considerations

They are needed on 32-bit platforms because of different calling conventions
and again in the units implementing AltiVec and Streams support.

gcc/ada/

* libgnat/g-alvevi.ads: Add pragma Universal_Aliasing for all the
view types.
* libgnat/s-stratt.ads: Likewise for Fat_Pointer type.

Diff:
---
 gcc/ada/libgnat/g-alvevi.ads | 11 +++
 gcc/ada/libgnat/s-stratt.ads |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/gcc/ada/libgnat/g-alvevi.ads b/gcc/ada/libgnat/g-alvevi.ads
index b2beac7284c7..b0f58790adf8 100644
--- a/gcc/ada/libgnat/g-alvevi.ads
+++ b/gcc/ada/libgnat/g-alvevi.ads
@@ -58,6 +58,7 @@ package GNAT.Altivec.Vector_Views is
type VUC_View is record
   Values : Varray_unsigned_char;
end record;
+   pragma Universal_Aliasing (VUC_View);
 
type Varray_signed_char is array (Vchar_Range) of signed_char;
for Varray_signed_char'Alignment use VECTOR_ALIGNMENT;
@@ -65,6 +66,7 @@ package GNAT.Altivec.Vector_Views is
type VSC_View is record
   Values : Varray_signed_char;
end record;
+   pragma Universal_Aliasing (VSC_View);
 
type Varray_bool_char is array (Vchar_Range) of bool_char;
for Varray_bool_char'Alignment use VECTOR_ALIGNMENT;
@@ -72,6 +74,7 @@ package GNAT.Altivec.Vector_Views is
type VBC_View is record
   Values : Varray_bool_char;
end record;
+   pragma Universal_Aliasing (VBC_View);
 
--
-- short components --
@@ -85,6 +88,7 @@ package GNAT.Altivec.Vector_Views is
type VUS_View is record
   Values : Varray_unsigned_short;
end record;
+   pragma Universal_Aliasing (VUS_View);
 
type Varray_signed_short is array (Vshort_Range) of signed_short;
for Varray_signed_short'Alignment use VECTOR_ALIGNMENT;
@@ -92,6 +96,7 @@ package GNAT.Altivec.Vector_Views is
type VSS_View is record
   Values : Varray_signed_short;
end record;
+   pragma Universal_Aliasing (VSS_View);
 
type Varray_bool_short is array (Vshort_Range) of bool_short;
for Varray_bool_short'Alignment use VECTOR_ALIGNMENT;
@@ -99,6 +104,7 @@ package GNAT.Altivec.Vector_Views is
type VBS_View is record
   Values : Varray_bool_short;
end record;
+   pragma Universal_Aliasing (VBS_View);
 

-- int components --
@@ -112,6 +118,7 @@ package GNAT.Altivec.Vector_Views is
type VUI_View is record
   Values : Varray_unsigned_int;
end record;
+   pragma Universal_Aliasing (VUI_View);
 
type Varray_signed_int is array (Vint_Range) of signed_int;
for Varray_signed_int'Alignment use VECTOR_ALIGNMENT;
@@ -119,6 +126,7 @@ package GNAT.Altivec.Vector_Views is
type VSI_View is record
   Values : Varray_signed_int;
end record;
+   pragma Universal_Aliasing (VSI_View);
 
type Varray_bool_int is array (Vint_Range) of bool_int;
for Varray_bool_int'Alignment use VECTOR_ALIGNMENT;
@@ -126,6 +134,7 @@ package GNAT.Altivec.Vector_Views is
type VBI_View is record
   Values : Varray_bool_int;
end record;
+   pragma Universal_Aliasing (VBI_View);
 
--
-- float components --
@@ -139,6 +148,7 @@ package GNAT.Altivec.Vector_Views is
type VF_View is record
   Values : Varray_float;
end record;
+   pragma Universal_Aliasing (VF_View);
 
--
-- pixel components --
@@ -152,5 +162,6 @@ package GNAT.Altivec.Vector_Views is
type VP_View is record
   Values : Varray_pixel;
end record;
+   pragma Universal_Aliasing (VP_View);
 
 end GNAT.Altivec.Vector_Views;
diff --git a/gcc/ada/libgnat/s-stratt.ads b/gcc/ada/libgnat/s-stratt.ads
index 1d4c82d17abd..eee19f4bdced 100644
--- a/gcc/ada/libgnat/s-stratt.ads
+++ b/gcc/ada/libgnat/s-stratt.ads
@@ -74,6 +74,9 @@ package System.Stream_Attributes is
   P2 : System.Address;
end record;
 
+   pragma Universal_Aliasing (Fat_Pointer);
+   --  This avoids a copy for the aforementioned unchecked conversions
+

-- Treatment of enumeration types --



[PATCH 3/4] Libatomic: Clean up AArch64 ifunc aliasing

2024-05-16 Thread Victor Do Nascimento
Following improvements to the way ifuncs are selected based on
detected architectural features, we are able to do away with many of
the aliases that were previously needed for subsets of atomic
functions that were not implemented in a given extension.

This may be clarified by virtue of an example. Before, LSE128
functions carried the suffix _i1 and LSE2 functions the _i2.

Using a single ifunc selector for all atomic functions meant that if
LSE128 was detected, the _i1 function variant would be used
indiscriminately, irrespective of whether or not a function had an
LSE128-specific implementation.  Aliasing was thus needed to redirect
calls to these missing functions to their _i2 LSE2 alternatives.

The more architectural extensions for which support was added, the
more complex the aliasing chain.

With the per-file configuration of ifuncs, we do away with the need
for such aliasing.

libatomic/ChangeLog:

* config/linux/aarch64/atomic_16.S: Remove unnecessary
aliasing.
---
 libatomic/config/linux/aarch64/atomic_16.S | 41 --
 1 file changed, 41 deletions(-)

diff --git a/libatomic/config/linux/aarch64/atomic_16.S 
b/libatomic/config/linux/aarch64/atomic_16.S
index 1517e9e78df..16ff03057ab 100644
--- a/libatomic/config/linux/aarch64/atomic_16.S
+++ b/libatomic/config/linux/aarch64/atomic_16.S
@@ -732,47 +732,6 @@ ENTRY_ALIASED (test_and_set_16)
 END (test_and_set_16)
 
 
-/* Alias entry points which are the same in LSE2 and LSE128.  */
-
-#if HAVE_IFUNC
-# if !HAVE_FEAT_LSE128
-ALIAS (exchange_16, LSE128, LSE2)
-ALIAS (fetch_or_16, LSE128, LSE2)
-ALIAS (fetch_and_16, LSE128, LSE2)
-ALIAS (or_fetch_16, LSE128, LSE2)
-ALIAS (and_fetch_16, LSE128, LSE2)
-# endif
-ALIAS (load_16, LSE128, LSE2)
-ALIAS (store_16, LSE128, LSE2)
-ALIAS (compare_exchange_16, LSE128, LSE2)
-ALIAS (fetch_add_16, LSE128, LSE2)
-ALIAS (add_fetch_16, LSE128, LSE2)
-ALIAS (fetch_sub_16, LSE128, LSE2)
-ALIAS (sub_fetch_16, LSE128, LSE2)
-ALIAS (fetch_xor_16, LSE128, LSE2)
-ALIAS (xor_fetch_16, LSE128, LSE2)
-ALIAS (fetch_nand_16, LSE128, LSE2)
-ALIAS (nand_fetch_16, LSE128, LSE2)
-ALIAS (test_and_set_16, LSE128, LSE2)
-
-/* Alias entry points which are the same in baseline and LSE2.  */
-
-ALIAS (exchange_16, LSE2, CORE)
-ALIAS (fetch_add_16, LSE2, CORE)
-ALIAS (add_fetch_16, LSE2, CORE)
-ALIAS (fetch_sub_16, LSE2, CORE)
-ALIAS (sub_fetch_16, LSE2, CORE)
-ALIAS (fetch_or_16, LSE2, CORE)
-ALIAS (or_fetch_16, LSE2, CORE)
-ALIAS (fetch_and_16, LSE2, CORE)
-ALIAS (and_fetch_16, LSE2, CORE)
-ALIAS (fetch_xor_16, LSE2, CORE)
-ALIAS (xor_fetch_16, LSE2, CORE)
-ALIAS (fetch_nand_16, LSE2, CORE)
-ALIAS (nand_fetch_16, LSE2, CORE)
-ALIAS (test_and_set_16, LSE2, CORE)
-#endif
-
 /* GNU_PROPERTY_AARCH64_* macros from elf.h for use in asm code.  */
 #define FEATURE_1_AND 0xc000
 #define FEATURE_1_BTI 1
-- 
2.34.1



[PATCH 0/4] Libatomic: Cleanup ifunc selector and aliasing

2024-05-16 Thread Victor Do Nascimento
The recent introduction of the optional LSE128 and RCPC3 architectural
extensions to AArch64 has further led to the increased flexibility of
atomic support in the architecture, with many extensions providing
support for distinct atomic operations, each with different potential
applications in mind.

This has led to maintenance difficulties in Libatomic, in particular
regarding the way the ifunc selector is generated via a series of
macro expansions at compile-time.

Until now, irrespective of the atomic operation in question, all atomic
functions for a particular operand size were expected to have the same
number of ifunc alternatives, meaning that a one-size-fits-all
approach could reasonably be taken for the selector.

This meant that if, hypothetically, for a particular architecture and
operand size one particular atomic operation was to have 3 different
implementations associated with different extensions, libatomic would
likewise be required to present three ifunc alternatives for all other
atomic functions.

The consequence in the design choice was the unnecessary use of
function aliasing and the unwieldy code which resulted from this.

This patch series attempts to remediate this issue by making the
preprocessor macros defining the number of ifunc alternatives and
their respective selection functions dependent on the file importing
the ifunc selector-generating framework.

all files are given `LAT_' macros, defined at the beginning
and undef'd at the end of the file.  It is these macros that are
subsequently used to fine-tune the behaviors of `libatomic_i.h' and
`host-config.h'.

In particular, the definition of the `IFUNC_NCOND(N)' and
`IFUNC_COND_' macros in host-config.h can now be guarded behind
these new file-specific macros, which ultimately control what the
`GEN_SELECTOR(X)' macro in `libatomic_i.h' expands to.  As both of
these headers are imported once per file implementing some atomic
operation, fine-tuned control is now possible.

Regtested with both `--enable-gnu-indirect-function' and
`--disable-gnu-indirect-function' configurations on armv9.4-a target
with LRCPC3 and LSE128 support and without.

Victor Do Nascimento (4):
  Libatomic: Define per-file identifier macros
  Libatomic: Make ifunc selector behavior contingent on importing file
  Libatomic: Clean up AArch64 ifunc aliasing
  Libatomic: Clean up AArch64 `atomic_16.S' implementation file

 libatomic/cas_n.c|   2 +
 libatomic/config/linux/aarch64/atomic_16.S   | 623 +--
 libatomic/config/linux/aarch64/host-config.h |  35 +-
 libatomic/exch_n.c   |   2 +
 libatomic/fadd_n.c   |   2 +
 libatomic/fand_n.c   |   2 +
 libatomic/fence.c|   2 +
 libatomic/fenv.c |   2 +
 libatomic/fior_n.c   |   2 +
 libatomic/flag.c |   2 +
 libatomic/fnand_n.c  |   2 +
 libatomic/fop_n.c|   2 +
 libatomic/fsub_n.c   |   2 +
 libatomic/fxor_n.c   |   2 +
 libatomic/gcas.c |   2 +
 libatomic/gexch.c|   2 +
 libatomic/glfree.c   |   2 +
 libatomic/gload.c|   2 +
 libatomic/gstore.c   |   2 +
 libatomic/load_n.c   |   2 +
 libatomic/store_n.c  |   2 +
 libatomic/tas_n.c|   2 +
 22 files changed, 357 insertions(+), 341 deletions(-)

-- 
2.34.1



[Bug c/115096] -fstrict-aliasing miscompilation or missing warning

2024-05-14 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115096

--- Comment #2 from Andrew Pinski  ---
> the warning described in that bug is not occurring. 

That is because in that case it had literally `(unsigned char**)` while in
this case it is spread across function calls and that would require a lot of
work to get the warning happening.

Note there are a few open bug reports specifically asking for a runtime
catching of this too; but that is also hard as you need to record the aliasing
set of when stores and loads happen.

[Bug c/115096] -fstrict-aliasing miscompilation or missing warning

2024-05-14 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115096

Andrew Pinski  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |INVALID

--- Comment #1 from Andrew Pinski  ---
So this is not a miscompiling as the pointer types "char*" and "unsigned char*"
are two distant pointer types and are not considered to be aliasing and you are
not accessing the pointer via a character type (GCC has an exception for void*
types though which you could do the store in and get the correct result).

There is no warning here because the casting between pointer types is not the
issue but rather the access.

Since test is passed a pointer to a "char*" type and only in test you do the
cast to a pointer to a "unsigned char*" type. a warning in this case would 100%
require full path analysis of the code.

Note you could rewrite the code to be:
```
static int helper(void **cursor)
{
// first character ('a') is OK
if (*(unsigned char*)*cursor > 'z')
{
return -11;
}
*cursor = ((unsigned char*)*cursor) + 1;
// second character ('b') is OK
if (*(unsigned char*)*cursor > 'z')
{
return -12;
}
// increment to third character ('.')...
*cursor = ((unsigned char*)*cursor) + 1;
return 0;
}
```

And it would work correctly as I mentioned GCC has an exception on the `void*`
type which is allowed to alias all other pointer types. (Note I am not 100%
sure if C standard says that is still undefined code or not but GCC makes it as
such).

[Bug c/115096] New: -fstrict-aliasing miscompilation or missing warning

2024-05-14 Thread Philip.Taff at itron dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115096

Bug ID: 115096
   Summary: -fstrict-aliasing miscompilation or missing warning
   Product: gcc
   Version: 12.3.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: Philip.Taff at itron dot com
  Target Milestone: ---

Created attachment 58209
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58209=edit
minimal test case

I'm not sure whether this code is a violation of strict-aliasing, in which case
a warning would be great, or it is valid, in which case it is being
miscompiled, removing some of the increments. According to Bug #29901 a similar
cast does break strict-aliasing rules, but the warning described in that bug is
not occurring. 

Built with:
$ gcc -O1 -fstrict-aliasing -Wall -Wextra test.c

No compiler warnings or errors.

Executed with: ("ab." is the test data)
$ ./a.out ab.

GCC/system information:
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc-12
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/12/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
12.3.0-1ubuntu1~22.04' --with-bugurl=file:///usr/share/doc/gcc-12/README.Bugs
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-12
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug
--enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new
--enable-gnu-unique-object --disable-vtable-verify --enable-plugin
--enable-default-pie --with-system-zlib --enable-libphobos-checking=release
--with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch
--disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64
--with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none=/build/gcc-12-ALHxjy/gcc-12-12.3.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-12-ALHxjy/gcc-12-12.3.0/debian/tmp-gcn/usr
--enable-offload-defaulted --without-cuda-driver --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.3.0 (Ubuntu 12.3.0-1ubuntu1~22.04)
COLLECT_GCC_OPTIONS='-O1' '-fstrict-aliasing' '-Wall' '-Wextra' '-v'
'-save-temps' '-mtune=generic' '-march=x86-64' '-dumpdir' 'a-'
/usr/lib/gcc/x86_64-linux-gnu/12/cc1 -E -quiet -v -imultiarch x86_64-linux-gnu
test.c -mtune=generic -march=x86-64 -Wall -Wextra -fstrict-aliasing -O1
-fpch-preprocess -fasynchronous-unwind-tables -fstack-protector-strong
-Wformat-security -fstack-clash-protection -fcf-protection -o a-test.i
ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu"
ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/12/include-fixed"
ignoring nonexistent directory
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
/usr/lib/gcc/x86_64-linux-gnu/12/include
/usr/local/include
/usr/include/x86_64-linux-gnu
/usr/include
End of search list.
COLLECT_GCC_OPTIONS='-O1' '-fstrict-aliasing' '-Wall' '-Wextra' '-v'
'-save-temps' '-mtune=generic' '-march=x86-64' '-dumpdir' 'a-'
/usr/lib/gcc/x86_64-linux-gnu/12/cc1 -fpreprocessed a-test.i -quiet -dumpdir a-
-dumpbase test.c -dumpbase-ext .c -mtune=generic -march=x86-64 -O1 -Wall
-Wextra -version -fstrict-aliasing -fasynchronous-unwind-tables
-fstack-protector-strong -Wformat-security -fstack-clash-protection
-fcf-protection -o a-test.s
GNU C17 (Ubuntu 12.3.0-1ubuntu1~22.04) version 12.3.0 (x86_64-linux-gnu)
compiled by GNU C version 12.3.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C17 (Ubuntu 12.3.0-1ubuntu1~22.04) version 12.3.0 (x86_64-linux-gnu)
compiled by GNU C version 12.3.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: d93534411cccf908c6f3926ba41cb359
COLLECT_GCC_OPTIONS='-O1' '-fstrict-aliasing' '-Wall' '-Wextra' '-v'
'-save-temps' '-mtune=generic' '-march=x86-64' '-dumpdir' 'a-'
as -v --64 -o a-test.o a-test.s
GNU assembler version 2.38 (x86_64-linux-gnu) using BFD version (GNU Binutils
for Ubuntu) 2.38
COMPILER_PATH=/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-g

[Bug lto/112732] during IPA pass: *free_lang_data ICE: 'verify_type' failed: type variant with 'TYPE_ALIAS_SET_KNOWN_P' with -Os -Wstrict-aliasing=2 -flto

2024-05-08 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112732

--- Comment #5 from GCC Commits  ---
The releases/gcc-13 branch has been updated by Richard Biener
:

https://gcc.gnu.org/g:664ab9c6e8a5d031031596100997e025e5334e86

commit r13-8721-g664ab9c6e8a5d031031596100997e025e5334e86
Author: Richard Biener 
Date:   Tue Nov 28 12:36:21 2023 +0100

middle-end/112732 - stray TYPE_ALIAS_SET in type variant

The following fixes a stray TYPE_ALIAS_SET in a type variant built
by build_opaque_vector_type which is diagnosed by type checking
enabled with -flto.

PR middle-end/112732
* tree.cc (build_opaque_vector_type): Reset TYPE_ALIAS_SET
of the newly built type.

(cherry picked from commit f26d68d5d128c86faaceeb81b1e8f22254ad53df)

[Bug tree-optimization/113630] [11/12 Regression] -fno-strict-aliasing introduces out-of-bounds memory access

2024-05-06 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630

Richard Biener  changed:

   What|Removed |Added

 CC||rguenth at gcc dot gnu.org
  Known to fail||13.2.0
 Status|ASSIGNED|NEW
  Known to work||13.2.1
Summary|[11/12/13 Regression]   |[11/12 Regression]
   |-fno-strict-aliasing|-fno-strict-aliasing
   |introduces out-of-bounds|introduces out-of-bounds
   |memory access   |memory access
   Assignee|rguenth at gcc dot gnu.org |unassigned at gcc dot 
gnu.org

--- Comment #9 from Richard Biener  ---
Backported as far as I want.

[Bug tree-optimization/113630] [11/12/13 Regression] -fno-strict-aliasing introduces out-of-bounds memory access

2024-05-06 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630

--- Comment #8 from GCC Commits  ---
The releases/gcc-13 branch has been updated by Richard Biener
:

https://gcc.gnu.org/g:47cd06042237bf2d4f05b8355362bc038f6fa445

commit r13-8693-g47cd06042237bf2d4f05b8355362bc038f6fa445
Author: Richard Biener 
Date:   Wed Jan 31 11:28:50 2024 +0100

tree-optimization/113630 - invalid code hoisting

The following avoids code hoisting (but also PRE insertion) of
expressions that got value-numbered to another one that are not
a valid replacement (but still compute the same value).  This time
because the access path ends in a structure with different size,
meaning we consider a related access as not trapping because of the
size of the base of the access.

PR tree-optimization/113630
* tree-ssa-pre.cc (compute_avail): Avoid registering a
reference with a representation with not matching base
access size.

* gcc.dg/torture/pr113630.c: New testcase.

(cherry picked from commit 724b64304ff5c8ac08a913509afd6fde38d7b767)

[PATCH] c++, v5: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-25 Thread Jakub Jelinek
On Thu, Apr 25, 2024 at 11:30:48AM -0400, Jason Merrill wrote:
> Hmm, maybe maybe_clone_body shouldn't clear DECL_SAVED_TREE for aliases, but
> rather set it to some stub like void_node?

I'll try that in stage1.

> Though with all these changes, it's probably better to go with your first
> patch for GCC 14 and delay this approach to 15.  Your v1 patch is OK for 14.

Just to record, following patch passed bootstrap/regtest on x86_64-linux and
i686-linux.  But I've committed the v1 version instead with the addition of
comdat2.C and comdat5.C testcases from this patch now and in stage1 will
post an incremental diff.

Thanks.

2024-04-25  Jakub Jelinek  
Jason Merrill  

PR lto/113208
* decl2.cc (tentative_decl_linkage): Call maybe_make_one_only
for implicit instantiations of maybe in charge ctors/dtors
declared inline.
(c_parse_final_cleanups): Don't skip used same body aliases which
have non-NULL DECL_SAVED_TREE on the alias target.  Formatting fixes.
* optimize.cc (can_alias_cdtor): Adjust condition, for
HAVE_COMDAT_GROUP && DECL_ONE_ONLY && DECL_WEAK return true even
if not DECL_INTERFACE_KNOWN.
* decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P
functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already
set.

* g++.dg/abi/comdat2.C: New test.
* g++.dg/abi/comdat3.C: New test.
* g++.dg/abi/comdat4.C: New test.
* g++.dg/abi/comdat5.C: New test.
* g++.dg/lto/pr113208_0.C: New test.
* g++.dg/lto/pr113208_1.C: New file.
* g++.dg/lto/pr113208.h: New file.

--- gcc/cp/decl2.cc.jj  2024-04-24 18:28:22.299513620 +0200
+++ gcc/cp/decl2.cc 2024-04-25 16:19:17.385547357 +0200
@@ -3312,16 +3312,23 @@ tentative_decl_linkage (tree decl)
 linkage of all functions, and as that causes writes to
 the data mapped in from the PCH file, it's advantageous
 to mark the functions at this point.  */
- if (DECL_DECLARED_INLINE_P (decl)
- && (!DECL_IMPLICIT_INSTANTIATION (decl)
- || DECL_DEFAULTED_FN (decl)))
+ if (DECL_DECLARED_INLINE_P (decl))
{
- /* This function must have external linkage, as
-otherwise DECL_INTERFACE_KNOWN would have been
-set.  */
- gcc_assert (TREE_PUBLIC (decl));
- comdat_linkage (decl);
- DECL_INTERFACE_KNOWN (decl) = 1;
+ if (!DECL_IMPLICIT_INSTANTIATION (decl)
+ || DECL_DEFAULTED_FN (decl))
+   {
+ /* This function must have external linkage, as
+otherwise DECL_INTERFACE_KNOWN would have been
+set.  */
+ gcc_assert (TREE_PUBLIC (decl));
+ comdat_linkage (decl);
+ DECL_INTERFACE_KNOWN (decl) = 1;
+   }
+ else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
+   /* For implicit instantiations of cdtors try to make
+  it comdat, so that maybe_clone_body can use aliases.
+  See PR113208.  */
+   maybe_make_one_only (decl);
}
}
   else if (VAR_P (decl))
@@ -5264,7 +5271,19 @@ c_parse_final_cleanups (void)
generate_tls_wrapper (decl);
 
  if (!DECL_SAVED_TREE (decl))
-   continue;
+   {
+ cgraph_node *node;
+ tree tgt;
+ /* Even when maybe_clone_body created same body alias
+has no DECL_SAVED_TREE, if its alias target does,
+don't skip it.  */
+ if (!DECL_CLONED_FUNCTION (decl)
+ || !(node = cgraph_node::get (decl))
+ || !node->cpp_implicit_alias
+ || !(tgt = node->get_alias_target_tree ())
+ || !DECL_SAVED_TREE (tgt))
+   continue;
+   }
 
  cgraph_node *node = cgraph_node::get_create (decl);
 
@@ -5292,7 +5311,7 @@ c_parse_final_cleanups (void)
node = node->get_alias_target ();
 
  node->call_for_symbol_thunks_and_aliases (clear_decl_external,
- NULL, true);
+   NULL, true);
  /* If we mark !DECL_EXTERNAL one of the symbols in some comdat
 group, we need to mark all symbols in the same comdat group
 that way.  */
@@ -5302,7 +5321,7 @@ c_parse_final_cleanups (void)
 next != node;
 next = dyn_cast (next->same_comdat_group))
  next->call_for_symbol_thunks_and_aliases (clear_decl_external,
- NULL, true);
+   NULL, true);
}
 
  /* If we're going to need to write this 

[gcc r14-10132] c++: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-25 Thread Jakub Jelinek via Gcc-cvs
https://gcc.gnu.org/g:c39654e7a431992773b48d61f804494b0d70855f

commit r14-10132-gc39654e7a431992773b48d61f804494b0d70855f
Author: Jakub Jelinek 
Date:   Thu Apr 25 20:37:10 2024 +0200

c++: Retry the aliasing of base/complete cdtor optimization at 
import_export_decl time [PR113208]

When expand_or_defer_fn is called at_eof time, it calls import_export_decl
and then maybe_clone_body, which uses DECL_ONE_ONLY and comdat name in a
couple of places to try to optimize cdtors which are known to have the
same body by making the complete cdtor an alias to base cdtor (and in
that case also uses *[CD]5* as comdat group name instead of the normal
comdat group names specific to each mangled name).
Now, this optimization depends on DECL_ONE_ONLY and DECL_INTERFACE_KNOWN,
maybe_clone_body and can_alias_cdtor use:
  if (DECL_ONE_ONLY (fn))
cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group 
(clone));
...
  bool can_alias = can_alias_cdtor (fn);
...
  /* Tell cgraph if both ctors or both dtors are known to have
 the same body.  */
  if (can_alias
  && fns[0]
  && idx == 1
  && cgraph_node::get_create (fns[0])->create_same_body_alias
   (clone, fns[0]))
{
  alias = true;
  if (DECL_ONE_ONLY (fns[0]))
{
  /* For comdat base and complete cdtors put them
 into the same, *[CD]5* comdat group instead of
 *[CD][12]*.  */
  comdat_group = cdtor_comdat_group (fns[1], fns[0]);
  cgraph_node::get_create (fns[0])->set_comdat_group 
(comdat_group);
  if (symtab_node::get (clone)->same_comdat_group)
symtab_node::get (clone)->remove_from_same_comdat_group ();
  symtab_node::get (clone)->add_to_same_comdat_group
(symtab_node::get (fns[0]));
}
}
and
  /* Don't use aliases for weak/linkonce definitions unless we can put both
 symbols in the same COMDAT group.  */
  return (DECL_INTERFACE_KNOWN (fn)
  && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn))
  && (!DECL_ONE_ONLY (fn)
  || (HAVE_COMDAT_GROUP && DECL_WEAK (fn;
The following testcase regressed with Marek's r14-5979 change,
when pr113208_0.C is compiled where the ctor is marked constexpr,
we no longer perform this optimization, where
_ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
_ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
_ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
_ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
_ZN6vectorI12QualityValueEC1ERKS1_ comdat group.
Now, the linker seems to somehow cope with that, eventhough it
probably keeps both copies of the ctor, but seems LTO can't cope
with that and Honza doesn't know what it should do in that case
(linker decides that the prevailing symbol is
_ZN6vectorI12QualityValueEC2ERKS1_ (from the
_ZN6vectorI12QualityValueEC2ERKS1_ comdat group) and
_ZN6vectorI12QualityValueEC1ERKS1_ alias (from the other TU,
from _ZN6vectorI12QualityValueEC5ERKS1_ comdat group)).

Note, the case where some constructor is marked constexpr in one
TU and not in another one happens pretty often in libstdc++ when
one mixes -std= flags used to compile different compilation units.

The reason the optimization doesn't trigger when the constructor is
constexpr is that expand_or_defer_fn is called in that case much earlier
than when it is not constexpr; in the former case it is called when we
try to constant evaluate that constructor.  But DECL_INTERFACE_KNOWN
is false in that case and comdat_linkage hasn't been called either
(I think it is desirable, because comdat group is stored in the cgraph
node and am not sure it is a good idea to create cgraph nodes for
something that might not be needed later on at all), so maybe_clone_body
clones the bodies, but doesn't make them as aliases.

The following patch is an attempt to redo this optimization when
import_export_decl is called at_eof time on the base/complete cdtor
(or deleting dtor).  It will not do anything if maybe_clone_body
hasn't been called uyet (the TREE_ASM_WRITTEN check on the
DECL_MAYBE_IN_CHARGE_CDTOR_P), or when one or both of the base/complete
cdtors have been lowered already, or when maybe_clone_body called
maybe_thunk_body and it was successful.  Otherwise retries the
can_alias_cdtor check and makes the complete cdtor alias to the
base cdtor with a

Re: [PATCH] c++, v4: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-25 Thread Jason Merrill

On 4/25/24 07:22, Jakub Jelinek wrote:

On Thu, Apr 25, 2024 at 02:02:32PM +0200, Jakub Jelinek wrote:

I've tried the following patch, but unfortunately that lead to large
number of regressions:
+FAIL: g++.dg/cpp0x/initlist25.C  -std=c++17 (test for excess errors)


So the reduced testcase for this is
template  struct A {
   T a1;
   U a2;
   template 
   constexpr A(V &, W &) : a1(x), a2(y) {}
};
template  struct B;
namespace std {
template  struct initializer_list {
   int *_M_array;
   decltype (sizeof 0) _M_len;
};
}
template  struct C {
   void foo (std::initializer_list>);
};
template  struct D;
template , typename = B>
struct E { E (const char *); ~E (); };
int
main ()
{
   C, E> m;
   m.foo ({{"t", "t"}, {"y", "y"}});
}
Without the patch I've just posted or even with the earlier version
of the patch the
_ZN1AIK1EIc1DIcE1BIcEES5_EC[12]IRA2_KcSB_Lb1EEEOT_OT0_
ctors were emitted, but with this patch they are unresolved externals.

The reason is that the code actually uses (calls) the
_ZN1AIK1EIc1DIcE1BIcEES5_EC1IRA2_KcSB_Lb1EEEOT_OT0_
__ct_comp constructor, that one has TREE_USED, while the
_ZN1AIK1EIc1DIcE1BIcEES5_EC2IRA2_KcSB_Lb1EEEOT_OT0_
__ct_base constructor is not TREE_USED.

But the c_parse_final_cleanups loop over
FOR_EACH_VEC_SAFE_ELT (deferred_fns, i, decl)
will ignore the TREE_USED __ct_comp because it is an alias
and so has !DECL_SAVED_TREE:
5273  if (!DECL_SAVED_TREE (decl))
5274continue;


Hmm, maybe maybe_clone_body shouldn't clear DECL_SAVED_TREE for aliases, 
but rather set it to some stub like void_node?


Though with all these changes, it's probably better to go with your 
first patch for GCC 14 and delay this approach to 15.  Your v1 patch is 
OK for 14.


Jason



Re: [PATCH] c++, v4: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-25 Thread Jakub Jelinek
On Thu, Apr 25, 2024 at 02:02:32PM +0200, Jakub Jelinek wrote:
> I've tried the following patch, but unfortunately that lead to large
> number of regressions:
> +FAIL: g++.dg/cpp0x/initlist25.C  -std=c++17 (test for excess errors)

So the reduced testcase for this is
template  struct A {
  T a1;
  U a2;
  template 
  constexpr A(V &, W &) : a1(x), a2(y) {}
};
template  struct B;
namespace std {
template  struct initializer_list {
  int *_M_array;
  decltype (sizeof 0) _M_len;
};
}
template  struct C {
  void foo (std::initializer_list>);
};
template  struct D;
template , typename = B>
struct E { E (const char *); ~E (); };
int
main ()
{
  C, E> m;
  m.foo ({{"t", "t"}, {"y", "y"}});
}
Without the patch I've just posted or even with the earlier version
of the patch the
_ZN1AIK1EIc1DIcE1BIcEES5_EC[12]IRA2_KcSB_Lb1EEEOT_OT0_
ctors were emitted, but with this patch they are unresolved externals.

The reason is that the code actually uses (calls) the
_ZN1AIK1EIc1DIcE1BIcEES5_EC1IRA2_KcSB_Lb1EEEOT_OT0_
__ct_comp constructor, that one has TREE_USED, while the
_ZN1AIK1EIc1DIcE1BIcEES5_EC2IRA2_KcSB_Lb1EEEOT_OT0_
__ct_base constructor is not TREE_USED.

But the c_parse_final_cleanups loop over
FOR_EACH_VEC_SAFE_ELT (deferred_fns, i, decl)
will ignore the TREE_USED __ct_comp because it is an alias
and so has !DECL_SAVED_TREE:
5273  if (!DECL_SAVED_TREE (decl))
5274continue;

With the following incremental patch the tests in make check-g++
(haven't tried the coroutine one) which failed with the earlier patch
now pass.

--- gcc/cp/decl2.cc.jj  2024-04-25 10:52:21.057535959 +0200
+++ gcc/cp/decl2.cc 2024-04-25 16:19:17.385547357 +0200
@@ -5271,7 +5271,19 @@ c_parse_final_cleanups (void)
generate_tls_wrapper (decl);
 
  if (!DECL_SAVED_TREE (decl))
-   continue;
+   {
+ cgraph_node *node;
+ tree tgt;
+ /* Even when maybe_clone_body created same body alias
+has no DECL_SAVED_TREE, if its alias target does,
+don't skip it.  */
+ if (!DECL_CLONED_FUNCTION (decl)
+ || !(node = cgraph_node::get (decl))
+ || !node->cpp_implicit_alias
+ || !(tgt = node->get_alias_target_tree ())
+ || !DECL_SAVED_TREE (tgt))
+   continue;
+   }
 
  cgraph_node *node = cgraph_node::get_create (decl);
 
@@ -5299,7 +5311,7 @@ c_parse_final_cleanups (void)
node = node->get_alias_target ();
 
  node->call_for_symbol_thunks_and_aliases (clear_decl_external,
- NULL, true);
+   NULL, true);
  /* If we mark !DECL_EXTERNAL one of the symbols in some comdat
 group, we need to mark all symbols in the same comdat group
 that way.  */
@@ -5309,7 +5321,7 @@ c_parse_final_cleanups (void)
 next != node;
 next = dyn_cast (next->same_comdat_group))
  next->call_for_symbol_thunks_and_aliases (clear_decl_external,
- NULL, true);
+   NULL, true);
}
 
  /* If we're going to need to write this function out, and


Jakub



[PATCH] c++, v4: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-25 Thread Jakub Jelinek
On Wed, Apr 24, 2024 at 08:43:46PM -0400, Jason Merrill wrote:
> > Then can_alias_cdtor would return false, because it ends with:
> >/* Don't use aliases for weak/linkonce definitions unless we can put both
> >   symbols in the same COMDAT group.  */
> >return (DECL_INTERFACE_KNOWN (fn)
> >&& (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn))
> >&& (!DECL_ONE_ONLY (fn)
> >|| (HAVE_COMDAT_GROUP && DECL_WEAK (fn;
> > Should we change that DECL_INTERFACE_KNOWN (fn) in there to
> > (DECL_INTERFACE_KNOWN (fn) || something) then and what that
> > something should be?  HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)?
> 
> Yes, I think reorganize to
> 
> ((DECL_INTERFACE_KNOWN (fn) && !DECL_WEAK (fn) && !DECL_ONE_ONLY (fn))
>  || (HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn))

I've tried the following patch, but unfortunately that lead to large
number of regressions:
+FAIL: g++.dg/coroutines/torture/co-yield-04-complex-local-state.C (test for 
excess errors)
+FAIL: g++.dg/coroutines/torture/func-params-08.C (test for excess errors)
+FAIL: g++.dg/coroutines/torture/func-params-09-awaitable-parms.C (test for 
excess errors)
+FAIL: g++.dg/cpp0x/constexpr-initlist.C  -std=c++11 (test for excess errors)
+FAIL: g++.dg/cpp0x/constexpr-initlist.C  -std=c++14 (test for excess errors)
+FAIL: g++.dg/cpp0x/constexpr-initlist.C  -std=c++17 (test for excess errors)
+FAIL: g++.dg/cpp0x/constexpr-initlist.C  -std=c++20 (test for excess errors)
+FAIL: g++.dg/cpp0x/constexpr-initlist.C  -std=c++23 (test for excess errors)
+FAIL: g++.dg/cpp0x/constexpr-initlist.C  -std=c++26 (test for excess errors)
+FAIL: g++.dg/cpp0x/initlist25.C  -std=c++11 (test for excess errors)
+FAIL: g++.dg/cpp0x/initlist25.C  -std=c++14 (test for excess errors)
+FAIL: g++.dg/cpp0x/initlist25.C  -std=c++17 (test for excess errors)
+FAIL: g++.dg/cpp0x/initlist25.C  -std=c++20 (test for excess errors)
+FAIL: g++.dg/cpp0x/initlist25.C  -std=c++23 (test for excess errors)
+FAIL: g++.dg/cpp0x/initlist25.C  -std=c++26 (test for excess errors)
+FAIL: g++.dg/cpp1y/pr95226.C  -std=c++20 (test for excess errors)
+FAIL: g++.dg/cpp1y/pr95226.C  -std=c++23 (test for excess errors)
+FAIL: g++.dg/cpp1y/pr95226.C  -std=c++26 (test for excess errors)
+FAIL: g++.dg/cpp1z/decomp12.C  -std=c++23 (test for excess errors)
+FAIL: g++.dg/cpp1z/decomp12.C  -std=c++26 (test for excess errors)
+FAIL: g++.dg/cpp1z/eval-order2.C  -std=c++20 (test for excess errors)
+FAIL: g++.dg/cpp1z/eval-order2.C  -std=c++23 (test for excess errors)
+FAIL: g++.dg/cpp1z/eval-order2.C  -std=c++26 (test for excess errors)
+FAIL: g++.dg/cpp2a/srcloc17.C  -std=c++20 (test for excess errors)
+FAIL: g++.dg/cpp2a/srcloc17.C  -std=c++23 (test for excess errors)
+FAIL: g++.dg/cpp2a/srcloc17.C  -std=c++26 (test for excess errors)
+FAIL: g++.old-deja/g++.jason/template31.C  -std=c++20 (test for excess errors)
+FAIL: g++.old-deja/g++.jason/template31.C  -std=c++23 (test for excess errors)
+FAIL: g++.old-deja/g++.jason/template31.C  -std=c++26 (test for excess errors)
+FAIL: 20_util/unique_ptr/creation/for_overwrite.cc  -std=gnu++26 (test for 
excess errors)
+FAIL: 23_containers/span/cons_1_assert_neg.cc  -std=gnu++20 (test for excess 
errors)
+FAIL: 23_containers/span/cons_1_assert_neg.cc  -std=gnu++26 (test for excess 
errors)
+FAIL: 23_containers/span/cons_2_assert_neg.cc  -std=gnu++20 (test for excess 
errors)
+FAIL: 23_containers/span/cons_2_assert_neg.cc  -std=gnu++26 (test for excess 
errors)
+FAIL: std/ranges/repeat/1.cc  -std=gnu++23 (test for excess errors)
+FAIL: std/ranges/repeat/1.cc  -std=gnu++26 (test for excess errors)

Errors are like:
func-params-08.C:(.text._ZNSt12_Vector_baseIiSaIiEEC2Ev[_ZNSt12_Vector_baseIiSaIiEEC5Ev]+0x14):
 undefined reference to 
`_ZNSt12_Vector_baseIiSaIiEE12_Vector_implC1EvQ26is_default_constructible_vIN9__gnu_cxx14__alloc_traitsIT0_NS5_10value_typeEE6rebindIT_E5otherEE'
Though, libstdc++.so.6 abilist is the same.
Trying to debug it now.

2024-04-24  Jakub Jelinek  
Jason Merrill  

PR lto/113208
* decl2.cc (tentative_decl_linkage): Call maybe_make_one_only
for implicit instantiations of maybe in charge ctors/dtors
declared inline.
* optimize.cc (can_alias_cdtor): Adjust condition, for
HAVE_COMDAT_GROUP && DECL_ONE_ONLY && DECL_WEAK return true even
if not DECL_INTERFACE_KNOWN.
* decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P
functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already
set.

* g++.dg/abi/comdat2.C: New test.
* g++.dg/abi/comdat3.C: New test.
* g++.dg/abi/comdat4.C: New test.
* g++.dg/abi/comdat5.C: New test.
* g++.dg/lto/pr113208_0.C: New test.
* g++.dg/lto/pr113208_1.C: New file.
* g++.dg/lto/pr113208.h: New file.

--- gcc/cp/decl2.cc.jj  2024-04-24 18:28:22.299513620 +0200
+++ gcc/cp/decl2.cc 2024-04-25 10:04:18.049476567 +0200
@@ -3312,16 +3312,23 @@ 

Re: [PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-24 Thread Jason Merrill

On 4/24/24 15:47, Jakub Jelinek wrote:

On Wed, Apr 24, 2024 at 06:39:33PM -0400, Jason Merrill wrote:

--- gcc/cp/decl2.cc.jj  2024-04-23 14:49:41.933186265 +0200
+++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200
@@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
 to mark the functions at this point.  */
  if (DECL_DECLARED_INLINE_P (decl)
  && (!DECL_IMPLICIT_INSTANTIATION (decl)
- || DECL_DEFAULTED_FN (decl)))
+ || DECL_DEFAULTED_FN (decl)
+ /* For implicit instantiations of cdtors,
+if import_export_decl would use comdat linkage,
+make sure to use it right away, so that maybe_clone_body
+can use aliases.  See PR113208.  */
+ || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
+ && (flag_implicit_templates
+ || flag_implicit_inline_templates)
+ && flag_weak
+ && TARGET_SUPPORTS_ALIASES)))


It seems wrong to set DECL_INTERFACE_KNOWN for cdtors that might get an
explicit instantiation later, and likewise for comdat_linkage when
!flag_weak; instead of adding this condition to the if, how about adding an
else like


   else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
 /* For implicit instantiations of cdtors,
if import_export_decl would use comdat linkage,
make sure to use it right away, so that maybe_clone_body
can use aliases.  See PR113208.  */
 maybe_make_one_only (decl);


?


Then can_alias_cdtor would return false, because it ends with:
   /* Don't use aliases for weak/linkonce definitions unless we can put both
  symbols in the same COMDAT group.  */
   return (DECL_INTERFACE_KNOWN (fn)
   && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn))
   && (!DECL_ONE_ONLY (fn)
   || (HAVE_COMDAT_GROUP && DECL_WEAK (fn;
Should we change that DECL_INTERFACE_KNOWN (fn) in there to
(DECL_INTERFACE_KNOWN (fn) || something) then and what that
something should be?  HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)?


Yes, I think reorganize to

((DECL_INTERFACE_KNOWN (fn) && !DECL_WEAK (fn) && !DECL_ONE_ONLY (fn))
 || (HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn))

Jason



Re: [PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-24 Thread Jakub Jelinek
On Wed, Apr 24, 2024 at 06:39:33PM -0400, Jason Merrill wrote:
> > --- gcc/cp/decl2.cc.jj  2024-04-23 14:49:41.933186265 +0200
> > +++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200
> > @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
> >  to mark the functions at this point.  */
> >   if (DECL_DECLARED_INLINE_P (decl)
> >   && (!DECL_IMPLICIT_INSTANTIATION (decl)
> > - || DECL_DEFAULTED_FN (decl)))
> > + || DECL_DEFAULTED_FN (decl)
> > + /* For implicit instantiations of cdtors,
> > +if import_export_decl would use comdat linkage,
> > +make sure to use it right away, so that maybe_clone_body
> > +can use aliases.  See PR113208.  */
> > + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
> > + && (flag_implicit_templates
> > + || flag_implicit_inline_templates)
> > + && flag_weak
> > + && TARGET_SUPPORTS_ALIASES)))
> 
> It seems wrong to set DECL_INTERFACE_KNOWN for cdtors that might get an
> explicit instantiation later, and likewise for comdat_linkage when
> !flag_weak; instead of adding this condition to the if, how about adding an
> else like
> 
> >   else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
> > /* For implicit instantiations of cdtors,
> > if import_export_decl would use comdat linkage,
> > make sure to use it right away, so that maybe_clone_body
> > can use aliases.  See PR113208.  */
> > maybe_make_one_only (decl);
> 
> ?

Then can_alias_cdtor would return false, because it ends with:
  /* Don't use aliases for weak/linkonce definitions unless we can put both
 symbols in the same COMDAT group.  */
  return (DECL_INTERFACE_KNOWN (fn)
  && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn))
  && (!DECL_ONE_ONLY (fn)
  || (HAVE_COMDAT_GROUP && DECL_WEAK (fn;
Should we change that DECL_INTERFACE_KNOWN (fn) in there to
(DECL_INTERFACE_KNOWN (fn) || something) then and what that
something should be?  HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)?

Jakub



Re: [PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-24 Thread Jason Merrill

On 4/24/24 09:16, Jakub Jelinek wrote:

On Wed, Apr 24, 2024 at 10:16:05AM +0100, Jonathan Wakely wrote:

That fixes the testcases too, but seems to regress
+FAIL: libstdc++-abi/abi_check



There are explicit instantiation definitions that should instantiate
those types:

src/c++17/fs_dir.cc:template class std::__shared_ptr;
src/c++17/fs_dir.cc:template class
std::__shared_ptr;
src/c++17/fs_path.cc:template class std::__shared_ptr;

So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o


So this boils down to (cvise reduced):
namespace __gnu_cxx { enum _Lock_policy { _S_single, _S_mutex, _S_atomic } 
const __default_lock_policy = _S_atomic; }
namespace std {
using __gnu_cxx::__default_lock_policy;
using __gnu_cxx::_Lock_policy;
template  struct __shared_ptr { 
constexpr __shared_ptr() {} };
namespace filesystem {
struct _Dir;
struct directory_iterator { __shared_ptr<_Dir> _M_dir; };
void end() { directory_iterator(); } }
extern template class __shared_ptr;
}
namespace fs = std::filesystem;
template class std::__shared_ptr;
at -O2, previously it used to emit
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE1EEC2Ev
but now no longer does with the yesterday posted PR113208 patch.

The following updated patch fixes that by calling note_vague_linkage_fn for
the cdtor clones from maybe_clone_body if the flags suggest that the
maybe-in-charge cdtor had tentative_decl_linkage -> note_vague_linkage_fn
called too.  And then I've noticed that in some cases the updated comdat
group set by maybe_clone_body (*C5* or *D5*) was then overwritten again by
maybe_make_one_only.  So the patch tweaks cxx_comdat_group, so that when
some comdat group has been chosen already it doesn't try to use some
different one.

Bootstrapped/regtested on x86_64-linux and i686-linux, this one doesn't
regress anything unlike the earlier patch.

2024-04-24  Jakub Jelinek  

PR lto/113208
* decl2.cc (tentative_decl_linkage): Use comdat_linkage also
for implicit instantiations of maybe in charge ctors/dtors
if -fimplicit-templates or -fimplicit-inline-templates and
-fweak and target supports aliases.
* optimize.cc (maybe_clone_body): Call note_vague_linkage_fn
on clone if fn has DECL_INTERFACE_KNOWN, DECL_NOT_REALLY_EXTERN
and DECL_DEFER_OUTPUT flags set.
* decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P
functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already
set.

* g++.dg/abi/comdat2.C: New test.
* g++.dg/abi/comdat3.C: New test.
* g++.dg/lto/pr113208_0.C: New test.
* g++.dg/lto/pr113208_1.C: New file.
* g++.dg/lto/pr113208.h: New file.

--- gcc/cp/decl2.cc.jj  2024-04-23 14:49:41.933186265 +0200
+++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200
@@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
 to mark the functions at this point.  */
  if (DECL_DECLARED_INLINE_P (decl)
  && (!DECL_IMPLICIT_INSTANTIATION (decl)
- || DECL_DEFAULTED_FN (decl)))
+ || DECL_DEFAULTED_FN (decl)
+ /* For implicit instantiations of cdtors,
+if import_export_decl would use comdat linkage,
+make sure to use it right away, so that maybe_clone_body
+can use aliases.  See PR113208.  */
+ || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
+ && (flag_implicit_templates
+ || flag_implicit_inline_templates)
+ && flag_weak
+ && TARGET_SUPPORTS_ALIASES)))


It seems wrong to set DECL_INTERFACE_KNOWN for cdtors that might get an 
explicit instantiation later, and likewise for comdat_linkage when 
!flag_weak; instead of adding this condition to the if, how about adding 
an else like



  else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
/* For implicit instantiations of cdtors,
   if import_export_decl would use comdat linkage,   
   make sure to use it right away, so that maybe_clone_body  
   can use aliases.  See PR113208.  */

maybe_make_one_only (decl);


?

Jason



[PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-24 Thread Jakub Jelinek
On Wed, Apr 24, 2024 at 10:16:05AM +0100, Jonathan Wakely wrote:
> > That fixes the testcases too, but seems to regress
> > +FAIL: libstdc++-abi/abi_check

> There are explicit instantiation definitions that should instantiate
> those types:
> 
> src/c++17/fs_dir.cc:template class std::__shared_ptr;
> src/c++17/fs_dir.cc:template class
> std::__shared_ptr;
> src/c++17/fs_path.cc:template class std::__shared_ptr fs::filesystem_error::_Impl>;
> 
> So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o

So this boils down to (cvise reduced):
namespace __gnu_cxx { enum _Lock_policy { _S_single, _S_mutex, _S_atomic } 
const __default_lock_policy = _S_atomic; }
namespace std {
using __gnu_cxx::__default_lock_policy;
using __gnu_cxx::_Lock_policy;
template  struct __shared_ptr { 
constexpr __shared_ptr() {} };
namespace filesystem {
struct _Dir;
struct directory_iterator { __shared_ptr<_Dir> _M_dir; };
void end() { directory_iterator(); } }
extern template class __shared_ptr;
}
namespace fs = std::filesystem;
template class std::__shared_ptr;
at -O2, previously it used to emit
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE1EEC2Ev
but now no longer does with the yesterday posted PR113208 patch.

The following updated patch fixes that by calling note_vague_linkage_fn for
the cdtor clones from maybe_clone_body if the flags suggest that the
maybe-in-charge cdtor had tentative_decl_linkage -> note_vague_linkage_fn
called too.  And then I've noticed that in some cases the updated comdat
group set by maybe_clone_body (*C5* or *D5*) was then overwritten again by
maybe_make_one_only.  So the patch tweaks cxx_comdat_group, so that when
some comdat group has been chosen already it doesn't try to use some
different one.

Bootstrapped/regtested on x86_64-linux and i686-linux, this one doesn't
regress anything unlike the earlier patch.

2024-04-24  Jakub Jelinek  

PR lto/113208
* decl2.cc (tentative_decl_linkage): Use comdat_linkage also
for implicit instantiations of maybe in charge ctors/dtors
if -fimplicit-templates or -fimplicit-inline-templates and
-fweak and target supports aliases.
* optimize.cc (maybe_clone_body): Call note_vague_linkage_fn
on clone if fn has DECL_INTERFACE_KNOWN, DECL_NOT_REALLY_EXTERN
and DECL_DEFER_OUTPUT flags set.
* decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P
functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already
set.

* g++.dg/abi/comdat2.C: New test.
* g++.dg/abi/comdat3.C: New test.
* g++.dg/lto/pr113208_0.C: New test.
* g++.dg/lto/pr113208_1.C: New file.
* g++.dg/lto/pr113208.h: New file.

--- gcc/cp/decl2.cc.jj  2024-04-23 14:49:41.933186265 +0200
+++ gcc/cp/decl2.cc 2024-04-24 15:17:09.043625729 +0200
@@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
 to mark the functions at this point.  */
  if (DECL_DECLARED_INLINE_P (decl)
  && (!DECL_IMPLICIT_INSTANTIATION (decl)
- || DECL_DEFAULTED_FN (decl)))
+ || DECL_DEFAULTED_FN (decl)
+ /* For implicit instantiations of cdtors,
+if import_export_decl would use comdat linkage,
+make sure to use it right away, so that maybe_clone_body
+can use aliases.  See PR113208.  */
+ || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
+ && (flag_implicit_templates
+ || flag_implicit_inline_templates)
+ && flag_weak
+ && TARGET_SUPPORTS_ALIASES)))
{
  /* This function must have external linkage, as
 otherwise DECL_INTERFACE_KNOWN would have been
--- gcc/cp/optimize.cc.jj   2024-04-24 13:44:26.456634100 +0200
+++ gcc/cp/optimize.cc  2024-04-24 14:46:13.050371557 +0200
@@ -717,6 +717,10 @@ maybe_clone_body (tree fn)
   else
expand_or_defer_fn (clone);
   first = false;
+  if (DECL_INTERFACE_KNOWN (fn)
+ && DECL_NOT_REALLY_EXTERN (fn)
+ && DECL_DEFER_OUTPUT (fn))
+   note_vague_linkage_fn (clone);
 }
   pop_from_top_level ();
 
--- gcc/cp/decl.cc.jj   2024-04-23 08:31:05.515161033 +0200
+++ gcc/cp/decl.cc  2024-04-24 15:15:34.401951491 +0200
@@ -19254,6 +19254,14 @@ cxx_comdat_group (tree decl)
  else
break;
}
+  /* If a ctor/dtor has already set the comdat group by
+maybe_clone_body, don't override it.  */
+  if (SUPPORTS_ONE_ONLY
+ && TREE_CODE (decl) == FUNCTION_DECL
+ && DECL_CLONED_FUNCTION_P (decl)
+ && SUPPORTS_ONE_ONLY)
+   if (tree comdat = DECL_COMDAT_GROUP (decl))
+ return comdat;
 }
 
   return decl;
--- gcc/testsuite/g++.dg/abi/comdat2.C.jj   2024-04-24 13:44:11.054849035 

Re: [PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-24 Thread Jonathan Wakely
On Tue, 23 Apr 2024 at 17:05, Jakub Jelinek  wrote:
>
> On Mon, Apr 22, 2024 at 11:14:35PM -0400, Jason Merrill wrote:
> > > > The following testcase regressed with Marek's r14-5979 change,
> > > > when pr113208_0.C is compiled where the ctor is marked constexpr,
> > > > we no longer perform this optimization, where
> > > > _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
> > > > _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
> > > > _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
> > > > instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
> > > > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
> > > > content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
> > > > _ZN6vectorI12QualityValueEC1ERKS1_ comdat group.
> >
> > This seems like an ABI bug that could use a non-LTO testcase.
>
> Well, except for the issues it causes to LTO I think it is compatible,
> worst case we get the body of the ctor duplicated in the executable
> and the linker picks some of the weak symbols as the symbol definitions.
> Anyway, I've added a non-LTO testcase for that in the patch below.
>
> > Hmm, cloning the bodies and then discarding them later seems like more extra
> > work than creating the cgraph nodes.
>
> So, I've tried to handle that in tentative_decl_linkage, like that function
> already handles functions declared inline except for implicit template
> instantiations.  If we expect that import_export_decl will do comdat_linkage
> for the ctor later on do it right away.
>
> That fixes the testcases too, but seems to regress
> +FAIL: libstdc++-abi/abi_check
> on both x86_64-linux and i686-linux, in each case 8 symbols disappeared from
> libstdc++.so.6:
> _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev
> _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev
> _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev
> _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
> _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev
> _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev
> _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev
> _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
>
> Will need to study why that happened, it might be that it was ok because
> I think the filesystem stuff is unlike the rest compiled with no exported
> templates, but would need at least some hacks in libstdc++ to preserve
> previously exported symbols.

There are explicit instantiation definitions that should instantiate
those types:

src/c++17/fs_dir.cc:template class std::__shared_ptr;
src/c++17/fs_dir.cc:template class
std::__shared_ptr;
src/c++17/fs_path.cc:template class std::__shared_ptr;

So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o



> Still, feels like a risky change this late if it wouldn't break ABI of other
> libraries.
>
> 2024-04-23  Jakub Jelinek  
>
> PR lto/113208
> * decl2.cc (tentative_decl_linkage): Use comdat_linkage also
> for implicit instantiations of maybe in charge ctors/dtors
> if -fimplicit-templates or -fimplicit-inline-templates and
> -fweak and target supports aliases.
>
> * g++.dg/abi/comdat2.C: New test.
> * g++.dg/lto/pr113208_0.C: New test.
> * g++.dg/lto/pr113208_1.C: New file.
> * g++.dg/lto/pr113208.h: New file.
>
> --- gcc/cp/decl2.cc.jj  2024-04-22 15:16:55.328548807 +0200
> +++ gcc/cp/decl2.cc 2024-04-23 09:52:18.993250442 +0200
> @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
>  to mark the functions at this point.  */
>   if (DECL_DECLARED_INLINE_P (decl)
>   && (!DECL_IMPLICIT_INSTANTIATION (decl)
> - || DECL_DEFAULTED_FN (decl)))
> + || DECL_DEFAULTED_FN (decl)
> + /* For implicit instantiations of cdtors,
> +if import_export_decl would use comdat linkage,
> +make sure to use it right away, so that maybe_clone_body
> +can use aliases.  See PR113208.  */
> + || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
> + && (flag_implicit_templates
> + || flag_implicit_inline_templates)
> + && flag_weak
> + && TARGET_SUPPORTS_ALIASES)))
> {
>   /* This function must have external linkage, as
>  otherwise DECL_INTERFACE_KNOWN would have been
> --- gcc/testsuite/g++.dg/abi/comdat2.C.jj   2024-04-23 10:04:28.485964610 
> +0200
> +++ gcc/testsuite/g++.dg/abi/comdat2.C  2024-04-23 10:05:24.757171194 +0200
> @@ -0,0 +1,26 

Re: [PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-23 Thread Jakub Jelinek
On Mon, Apr 22, 2024 at 11:14:35PM -0400, Jason Merrill wrote:
> > > The following testcase regressed with Marek's r14-5979 change,
> > > when pr113208_0.C is compiled where the ctor is marked constexpr,
> > > we no longer perform this optimization, where
> > > _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
> > > _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
> > > _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
> > > instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
> > > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
> > > content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
> > > _ZN6vectorI12QualityValueEC1ERKS1_ comdat group.
> 
> This seems like an ABI bug that could use a non-LTO testcase.

Well, except for the issues it causes to LTO I think it is compatible,
worst case we get the body of the ctor duplicated in the executable
and the linker picks some of the weak symbols as the symbol definitions.
Anyway, I've added a non-LTO testcase for that in the patch below.

> Hmm, cloning the bodies and then discarding them later seems like more extra
> work than creating the cgraph nodes.

So, I've tried to handle that in tentative_decl_linkage, like that function
already handles functions declared inline except for implicit template
instantiations.  If we expect that import_export_decl will do comdat_linkage
for the ctor later on do it right away.

That fixes the testcases too, but seems to regress
+FAIL: libstdc++-abi/abi_check
on both x86_64-linux and i686-linux, in each case 8 symbols disappeared from
libstdc++.so.6:
_ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev
_ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev
_ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev

Will need to study why that happened, it might be that it was ok because
I think the filesystem stuff is unlike the rest compiled with no exported
templates, but would need at least some hacks in libstdc++ to preserve
previously exported symbols.
Still, feels like a risky change this late if it wouldn't break ABI of other
libraries.

2024-04-23  Jakub Jelinek  

PR lto/113208
* decl2.cc (tentative_decl_linkage): Use comdat_linkage also
for implicit instantiations of maybe in charge ctors/dtors
if -fimplicit-templates or -fimplicit-inline-templates and
-fweak and target supports aliases.

* g++.dg/abi/comdat2.C: New test.
* g++.dg/lto/pr113208_0.C: New test.
* g++.dg/lto/pr113208_1.C: New file.
* g++.dg/lto/pr113208.h: New file.

--- gcc/cp/decl2.cc.jj  2024-04-22 15:16:55.328548807 +0200
+++ gcc/cp/decl2.cc 2024-04-23 09:52:18.993250442 +0200
@@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
 to mark the functions at this point.  */
  if (DECL_DECLARED_INLINE_P (decl)
  && (!DECL_IMPLICIT_INSTANTIATION (decl)
- || DECL_DEFAULTED_FN (decl)))
+ || DECL_DEFAULTED_FN (decl)
+ /* For implicit instantiations of cdtors,
+if import_export_decl would use comdat linkage,
+make sure to use it right away, so that maybe_clone_body
+can use aliases.  See PR113208.  */
+ || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
+ && (flag_implicit_templates
+ || flag_implicit_inline_templates)
+ && flag_weak
+ && TARGET_SUPPORTS_ALIASES)))
{
  /* This function must have external linkage, as
 otherwise DECL_INTERFACE_KNOWN would have been
--- gcc/testsuite/g++.dg/abi/comdat2.C.jj   2024-04-23 10:04:28.485964610 
+0200
+++ gcc/testsuite/g++.dg/abi/comdat2.C  2024-04-23 10:05:24.757171194 +0200
@@ -0,0 +1,26 @@
+// PR lto/113208
+// { dg-do compile { target { c++11 && { *-*-*gnu* } } } }
+// { dg-additional-options "-O2 -fkeep-inline-functions" }
+// { dg-final { scan-assembler "_ZN1BI1CEC5ERKS1_,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1BI1CEC1ERKS1_,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1BI1CEC2ERKS1_,comdat" } }
+
+template 
+struct A {
+  int foo () const;
+  A (int, int);
+};
+template 
+struct B : A {
+  constexpr B (const B ) : A (1, x.foo ()) {}
+  B () : A (1, 2) {}
+};
+struct C;
+struct D : B {};
+void 

[Committed] s390x: Fix vec_xl/vec_xst type aliasing [PR114676]

2024-04-23 Thread Andreas Krebbel
The requirements of the vec_xl/vec_xst intrinsincs wrt aliasing of the
pointer argument are not really documented.  As it turns out, users
are likely to get it wrong.  With this patch we let the pointer
argument alias everything in order to make it more robust for users.

Committed to mainline. Will be cherry-picked for stable branches as well.

gcc/ChangeLog:

PR target/114676
* config/s390/s390-c.cc (s390_expand_overloaded_builtin): Use a
MEM_REF with an addend of type ptr_type_node.

gcc/testsuite/ChangeLog:

PR target/114676
* gcc.target/s390/zvector/pr114676.c: New test.

Suggested-by: Jakub Jelinek 
---
 gcc/config/s390/s390-c.cc | 16 +---
 .../gcc.target/s390/zvector/pr114676.c| 19 +++
 2 files changed, 28 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/zvector/pr114676.c

diff --git a/gcc/config/s390/s390-c.cc b/gcc/config/s390/s390-c.cc
index 8d3d1a467a8..1bb6e810766 100644
--- a/gcc/config/s390/s390-c.cc
+++ b/gcc/config/s390/s390-c.cc
@@ -498,11 +498,11 @@ s390_expand_overloaded_builtin (location_t loc,
/* Build a vector type with the alignment of the source
   location in order to enable correct alignment hints to be
   generated for vl.  */
-   tree mem_type = build_aligned_type (return_type,
-   TYPE_ALIGN (TREE_TYPE (TREE_TYPE 
((*arglist)[1];
+   unsigned align = TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[1])));
+   tree mem_type = build_aligned_type (return_type, align);
return build2 (MEM_REF, mem_type,
   fold_build_pointer_plus ((*arglist)[1], (*arglist)[0]),
-  build_int_cst (TREE_TYPE ((*arglist)[1]), 0));
+  build_int_cst (ptr_type_node, 0));
   }
 case S390_OVERLOADED_BUILTIN_s390_vec_xst:
 case S390_OVERLOADED_BUILTIN_s390_vec_xstd2:
@@ -511,11 +511,13 @@ s390_expand_overloaded_builtin (location_t loc,
/* Build a vector type with the alignment of the target
   location in order to enable correct alignment hints to be
   generated for vst.  */
-   tree mem_type = build_aligned_type (TREE_TYPE((*arglist)[0]),
-   TYPE_ALIGN (TREE_TYPE (TREE_TYPE 
((*arglist)[2];
+   unsigned align = TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[2])));
+   tree mem_type = build_aligned_type (TREE_TYPE ((*arglist)[0]), align);
return build2 (MODIFY_EXPR, mem_type,
-  build1 (INDIRECT_REF, mem_type,
-  fold_build_pointer_plus ((*arglist)[2], 
(*arglist)[1])),
+  build2 (MEM_REF, mem_type,
+  fold_build_pointer_plus ((*arglist)[2],
+   (*arglist)[1]),
+  build_int_cst (ptr_type_node, 0)),
   (*arglist)[0]);
   }
 case S390_OVERLOADED_BUILTIN_s390_vec_load_pair:
diff --git a/gcc/testsuite/gcc.target/s390/zvector/pr114676.c 
b/gcc/testsuite/gcc.target/s390/zvector/pr114676.c
new file mode 100644
index 000..bdc66b2920a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/zvector/pr114676.c
@@ -0,0 +1,19 @@
+/* { dg-do run { target { s390*-*-* } } } */
+/* { dg-options "-O3 -mzarch -march=z14 -mzvector" } */
+
+#include 
+
+void __attribute__((noinline)) foo (int *mem)
+{
+  vec_xst ((vector float){ 1.0f, 2.0f, 3.0f, 4.0f }, 0, (float*)mem);
+}
+
+int
+main ()
+{
+  int m[4] = { 0 };
+  foo (m);
+  if (m[3] == 0)
+__builtin_abort ();
+  return 0;
+}
-- 
2.44.0



[gcc r14-10087] s390x: Fix vec_xl/vec_xst type aliasing [PR114676]

2024-04-23 Thread Andreas Krebbel via Gcc-cvs
https://gcc.gnu.org/g:42189f21b22c43ac8ab46edf5f6a7b4d99bc86a5

commit r14-10087-g42189f21b22c43ac8ab46edf5f6a7b4d99bc86a5
Author: Andreas Krebbel 
Date:   Tue Apr 23 10:05:46 2024 +0200

s390x: Fix vec_xl/vec_xst type aliasing [PR114676]

The requirements of the vec_xl/vec_xst intrinsincs wrt aliasing of the
pointer argument are not really documented.  As it turns out, users
are likely to get it wrong.  With this patch we let the pointer
argument alias everything in order to make it more robust for users.

gcc/ChangeLog:

PR target/114676
* config/s390/s390-c.cc (s390_expand_overloaded_builtin): Use a
MEM_REF with an addend of type ptr_type_node.

gcc/testsuite/ChangeLog:

PR target/114676
* gcc.target/s390/zvector/pr114676.c: New test.

Suggested-by: Jakub Jelinek 

Diff:
---
 gcc/config/s390/s390-c.cc| 16 +---
 gcc/testsuite/gcc.target/s390/zvector/pr114676.c | 19 +++
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/gcc/config/s390/s390-c.cc b/gcc/config/s390/s390-c.cc
index 8d3d1a467a8..1bb6e810766 100644
--- a/gcc/config/s390/s390-c.cc
+++ b/gcc/config/s390/s390-c.cc
@@ -498,11 +498,11 @@ s390_expand_overloaded_builtin (location_t loc,
/* Build a vector type with the alignment of the source
   location in order to enable correct alignment hints to be
   generated for vl.  */
-   tree mem_type = build_aligned_type (return_type,
-   TYPE_ALIGN (TREE_TYPE (TREE_TYPE 
((*arglist)[1];
+   unsigned align = TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[1])));
+   tree mem_type = build_aligned_type (return_type, align);
return build2 (MEM_REF, mem_type,
   fold_build_pointer_plus ((*arglist)[1], (*arglist)[0]),
-  build_int_cst (TREE_TYPE ((*arglist)[1]), 0));
+  build_int_cst (ptr_type_node, 0));
   }
 case S390_OVERLOADED_BUILTIN_s390_vec_xst:
 case S390_OVERLOADED_BUILTIN_s390_vec_xstd2:
@@ -511,11 +511,13 @@ s390_expand_overloaded_builtin (location_t loc,
/* Build a vector type with the alignment of the target
   location in order to enable correct alignment hints to be
   generated for vst.  */
-   tree mem_type = build_aligned_type (TREE_TYPE((*arglist)[0]),
-   TYPE_ALIGN (TREE_TYPE (TREE_TYPE 
((*arglist)[2];
+   unsigned align = TYPE_ALIGN (TREE_TYPE (TREE_TYPE ((*arglist)[2])));
+   tree mem_type = build_aligned_type (TREE_TYPE ((*arglist)[0]), align);
return build2 (MODIFY_EXPR, mem_type,
-  build1 (INDIRECT_REF, mem_type,
-  fold_build_pointer_plus ((*arglist)[2], 
(*arglist)[1])),
+  build2 (MEM_REF, mem_type,
+  fold_build_pointer_plus ((*arglist)[2],
+   (*arglist)[1]),
+  build_int_cst (ptr_type_node, 0)),
   (*arglist)[0]);
   }
 case S390_OVERLOADED_BUILTIN_s390_vec_load_pair:
diff --git a/gcc/testsuite/gcc.target/s390/zvector/pr114676.c 
b/gcc/testsuite/gcc.target/s390/zvector/pr114676.c
new file mode 100644
index 000..bdc66b2920a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/zvector/pr114676.c
@@ -0,0 +1,19 @@
+/* { dg-do run { target { s390*-*-* } } } */
+/* { dg-options "-O3 -mzarch -march=z14 -mzvector" } */
+
+#include 
+
+void __attribute__((noinline)) foo (int *mem)
+{
+  vec_xst ((vector float){ 1.0f, 2.0f, 3.0f, 4.0f }, 0, (float*)mem);
+}
+
+int
+main ()
+{
+  int m[4] = { 0 };
+  foo (m);
+  if (m[3] == 0)
+__builtin_abort ();
+  return 0;
+}


Re: [PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-22 Thread Jason Merrill

On 4/22/24 08:42, Jakub Jelinek wrote:

On Wed, Apr 17, 2024 at 09:42:47AM +0200, Jakub Jelinek wrote:

When expand_or_defer_fn is called at_eof time, it calls import_export_decl
and then maybe_clone_body, which uses DECL_ONE_ONLY and comdat name in a
couple of places to try to optimize cdtors which are known to have the
same body by making the complete cdtor an alias to base cdtor (and in
that case also uses *[CD]5* as comdat group name instead of the normal
comdat group names specific to each mangled name).
Now, this optimization depends on DECL_ONE_ONLY and DECL_INTERFACE_KNOWN,
maybe_clone_body and can_alias_cdtor use:
   if (DECL_ONE_ONLY (fn))
 cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group 
(clone));
...
   bool can_alias = can_alias_cdtor (fn);
...
   /* Tell cgraph if both ctors or both dtors are known to have
  the same body.  */
   if (can_alias
   && fns[0]
   && idx == 1
   && cgraph_node::get_create (fns[0])->create_same_body_alias
(clone, fns[0]))
 {
   alias = true;
   if (DECL_ONE_ONLY (fns[0]))
 {
   /* For comdat base and complete cdtors put them
  into the same, *[CD]5* comdat group instead of
  *[CD][12]*.  */
   comdat_group = cdtor_comdat_group (fns[1], fns[0]);
   cgraph_node::get_create (fns[0])->set_comdat_group 
(comdat_group);
   if (symtab_node::get (clone)->same_comdat_group)
 symtab_node::get (clone)->remove_from_same_comdat_group ();
   symtab_node::get (clone)->add_to_same_comdat_group
 (symtab_node::get (fns[0]));
 }
 }
and
   /* Don't use aliases for weak/linkonce definitions unless we can put both
  symbols in the same COMDAT group.  */
   return (DECL_INTERFACE_KNOWN (fn)
   && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn))
   && (!DECL_ONE_ONLY (fn)
   || (HAVE_COMDAT_GROUP && DECL_WEAK (fn;
The following testcase regressed with Marek's r14-5979 change,
when pr113208_0.C is compiled where the ctor is marked constexpr,
we no longer perform this optimization, where
_ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
_ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
_ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
_ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
_ZN6vectorI12QualityValueEC1ERKS1_ comdat group.


This seems like an ABI bug that could use a non-LTO testcase.


Now, the linker seems to somehow cope with that, eventhough it
probably keeps both copies of the ctor, but seems LTO can't cope
with that and Honza doesn't know what it should do in that case
(linker decides that the prevailing symbol is
_ZN6vectorI12QualityValueEC2ERKS1_ (from the
_ZN6vectorI12QualityValueEC2ERKS1_ comdat group) and
_ZN6vectorI12QualityValueEC1ERKS1_ alias (from the other TU,
from _ZN6vectorI12QualityValueEC5ERKS1_ comdat group)).

Note, the case where some constructor is marked constexpr in one
TU and not in another one happens pretty often in libstdc++ when
one mixes -std= flags used to compile different compilation units.

The reason the optimization doesn't trigger when the constructor is
constexpr is that expand_or_defer_fn is called in that case much earlier
than when it is not constexpr; in the former case it is called when we
try to constant evaluate that constructor.  But DECL_INTERFACE_KNOWN
is false in that case and comdat_linkage hasn't been called either
(I think it is desirable, because comdat group is stored in the cgraph
node and am not sure it is a good idea to create cgraph nodes for
something that might not be needed later on at all), so maybe_clone_body
clones the bodies, but doesn't make them as aliases.


Hmm, cloning the bodies and then discarding them later seems like more 
extra work than creating the cgraph nodes.


Jason



[PATCH] c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

2024-04-22 Thread Jakub Jelinek
On Wed, Apr 17, 2024 at 09:42:47AM +0200, Jakub Jelinek wrote:
> When expand_or_defer_fn is called at_eof time, it calls import_export_decl
> and then maybe_clone_body, which uses DECL_ONE_ONLY and comdat name in a
> couple of places to try to optimize cdtors which are known to have the
> same body by making the complete cdtor an alias to base cdtor (and in
> that case also uses *[CD]5* as comdat group name instead of the normal
> comdat group names specific to each mangled name).
> Now, this optimization depends on DECL_ONE_ONLY and DECL_INTERFACE_KNOWN,
> maybe_clone_body and can_alias_cdtor use:
>   if (DECL_ONE_ONLY (fn))
> cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group 
> (clone));
> ...
>   bool can_alias = can_alias_cdtor (fn);
> ...
>   /* Tell cgraph if both ctors or both dtors are known to have
>  the same body.  */
>   if (can_alias
>   && fns[0]
>   && idx == 1
>   && cgraph_node::get_create (fns[0])->create_same_body_alias
>(clone, fns[0]))
> {
>   alias = true;
>   if (DECL_ONE_ONLY (fns[0]))
> {
>   /* For comdat base and complete cdtors put them
>  into the same, *[CD]5* comdat group instead of
>  *[CD][12]*.  */
>   comdat_group = cdtor_comdat_group (fns[1], fns[0]);
>   cgraph_node::get_create (fns[0])->set_comdat_group 
> (comdat_group);
>   if (symtab_node::get (clone)->same_comdat_group)
> symtab_node::get (clone)->remove_from_same_comdat_group ();
>   symtab_node::get (clone)->add_to_same_comdat_group
> (symtab_node::get (fns[0]));
> }
> }
> and
>   /* Don't use aliases for weak/linkonce definitions unless we can put both
>  symbols in the same COMDAT group.  */
>   return (DECL_INTERFACE_KNOWN (fn)
>   && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn))
>   && (!DECL_ONE_ONLY (fn)
>   || (HAVE_COMDAT_GROUP && DECL_WEAK (fn;
> The following testcase regressed with Marek's r14-5979 change,
> when pr113208_0.C is compiled where the ctor is marked constexpr,
> we no longer perform this optimization, where
> _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
> _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
> _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
> instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
> _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
> content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
> _ZN6vectorI12QualityValueEC1ERKS1_ comdat group.
> Now, the linker seems to somehow cope with that, eventhough it
> probably keeps both copies of the ctor, but seems LTO can't cope
> with that and Honza doesn't know what it should do in that case
> (linker decides that the prevailing symbol is
> _ZN6vectorI12QualityValueEC2ERKS1_ (from the
> _ZN6vectorI12QualityValueEC2ERKS1_ comdat group) and
> _ZN6vectorI12QualityValueEC1ERKS1_ alias (from the other TU,
> from _ZN6vectorI12QualityValueEC5ERKS1_ comdat group)).
> 
> Note, the case where some constructor is marked constexpr in one
> TU and not in another one happens pretty often in libstdc++ when
> one mixes -std= flags used to compile different compilation units.
> 
> The reason the optimization doesn't trigger when the constructor is
> constexpr is that expand_or_defer_fn is called in that case much earlier
> than when it is not constexpr; in the former case it is called when we
> try to constant evaluate that constructor.  But DECL_INTERFACE_KNOWN
> is false in that case and comdat_linkage hasn't been called either
> (I think it is desirable, because comdat group is stored in the cgraph
> node and am not sure it is a good idea to create cgraph nodes for
> something that might not be needed later on at all), so maybe_clone_body
> clones the bodies, but doesn't make them as aliases.
> 
> The following patch is an attempt to redo this optimization when
> import_export_decl is called at_eof time on the base/complete cdtor
> (or deleting dtor).  It will not do anything if maybe_clone_body
> hasn't been called uyet (the TREE_ASM_WRITTEN check on the
> DECL_MAYBE_IN_CHARGE_CDTOR_P), or when one or both of the base/complete
> cdtors have been lowered already, or when maybe_clone_body called
> maybe_thunk_body and it was successful.  Otherwise retries the
> can_alias_cdtor check and makes the complete cdtor alias to the
> base cdtor with adjustments to the comdat group.

Here is an updated version of the patch which changes
-  DECL_SAVED_TREE (fns[1]) = NULL_TREE;
+  release_function_body (fns[1]);
as suggested by Honza.

Bootstrapped/regtested on x86_64-linux and i686-linux again, ok for trunk?

2024-04-22  Jakub Jelinek  

PR lto/113208
* cp-tree.h (maybe_optimize_cdtor): Declare.
* decl2.cc (import_export_decl): Call it for cloned cdtors.
* optimize.cc 

  1   2   3   4   5   6   7   8   9   10   >