[Bug ipa/113359] [13/14 Regression] LTO miscompilation of ceph on aarch64 and x86_64

[jamborm at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359

--- Comment #26 from Martin Jambor  ---
This should be fixed on master, I'll backport the fix in a few weeks to at
least gcc-13 where it was reported.

[Bug ipa/113359] [13/14 Regression] LTO miscompilation of ceph on aarch64 and x86_64

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359

--- Comment #25 from GCC Commits  ---
The master branch has been updated by Martin Jambor :

https://gcc.gnu.org/g:1e3312a25a7b34d6e3f549273e1674c7114e4408

commit r14-9841-g1e3312a25a7b34d6e3f549273e1674c7114e4408
Author: Martin Jambor 
Date:   Mon Apr 8 18:53:23 2024 +0200

ICF: Make ICF and SRA agree on padding

PR 113359 shows that (at least with -fno-strict-aliasing) ICF can
unify two functions which copy an aggregate type of the same size but
then SRA, through its total scalarization, can copy the aggregate by
pieces, skipping paddding, but the padding was not the same in the two
original functions that ICF unified.

This patch enhances SRA with the ability to collect padding
information which then can be compared from within ICF.  Unfortunately
SRA uses OPTION_SET_P when determining its limits, so ICF needs to
switch cfuns at least once to figure it out too.

gcc/ChangeLog:

2024-03-27  Martin Jambor  

PR ipa/113359
* ipa-icf-gimple.h (func_checker): New members
safe_for_total_scalarization_p, m_total_scalarization_limit_known_p
and m_total_scalarization_limit.
(func_checker::func_checker): Initialize new member variables.
* ipa-icf-gimple.cc: Include tree-sra.h.
(func_checker::func_checker): Initialize new member variables.
(func_checker::safe_for_total_scalarization_p): New function.
(func_checker::compare_operand): Use the new function.
* tree-sra.h (sra_get_max_scalarization_size): Declare.
(sra_total_scalarization_would_copy_same_data_p): Likewise.
* tree-sra.cc (prepare_iteration_over_array_elts): New function.
(class sra_padding_collecting): New.
(sra_padding_collecting::record_padding): Likewise.
(scalarizable_type_p): Rename to totally_scalarizable_type_p.  Add
ability to record padding when requested.
(totally_scalarize_subtree): Split out gathering information
necessary
to iterate over array elements to
prepare_iteration_over_array_elts.
Fix errornous early exit.
(analyze_all_variable_accesses): Adjust the call to
totally_scalarizable_type_p.  Move determining of total
scalariation
size limit...
(sra_get_max_scalarization_size): ...here.
(check_ts_and_push_padding_to_vec): New function.
(sra_total_scalarization_would_copy_same_data_p): Likewise.

gcc/testsuite/ChangeLog:

2024-03-27  Martin Jambor  

PR ipa/113359
* gcc.dg/lto/pr113359-1_0.c: New.
* gcc.dg/lto/pr113359-1_1.c: Likewise.
* gcc.dg/lto/pr113359-2_0.c: Likewise.
* gcc.dg/lto/pr113359-2_1.c: Likewise.
* gcc.dg/lto/pr113359-3_0.c: Likewise.
* gcc.dg/lto/pr113359-3_1.c: Likewise.
* gcc.dg/lto/pr113359-4_0.c: Likewise.
* gcc.dg/lto/pr113359-4_1.c: Likewise.
* gcc.dg/lto/pr113359-5_0.c: Likewise.
* gcc.dg/lto/pr113359-5_1.c: Likewise.

[Bug ipa/113359] [13/14 Regression] LTO miscompilation of ceph on aarch64 and x86_64

[jamborm at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359

--- Comment #24 from Martin Jambor  ---
(In reply to Jan Hubicka from comment #23)
> I however wonder if we really guarantee to copy the paddings everywhere else
> then the total scalarization part?
> (i.e. in all paths through the RTL expansion)

I wanted that we sometimes don't do that in PR 80689 and the idea was
refused.  And as far as I can recall the code I don't think we do.

Anyway, I have sent the patch to the mailing list:
https://inbox.sourceware.org/gcc-patches/ri6jzlc25db@virgil.suse.cz/T/#u

[Bug ipa/113359] [13/14 Regression] LTO miscompilation of ceph on aarch64 and x86_64

[hubicka at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359

--- Comment #23 from Jan Hubicka  ---
The patch looks reasonable.  We probably could hash the padding vectors at
summary generation time to reduce WPA overhead, but that can be done
incrementally next stage1.
I however wonder if we really guarantee to copy the paddings everywhere else
then the total scalarization part?
(i.e. in all paths through the RTL expansion)

[Bug ipa/113359] [13/14 Regression] LTO miscompilation of ceph on aarch64 and x86_64

[jamborm at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359

--- Comment #22 from Martin Jambor  ---
Created attachment 57828
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57828=edit
Potential fix

I'm testing this patch

[Bug ipa/113359] [13/14 Regression] LTO miscompilation of ceph on aarch64 and x86_64

[rguenth at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359

Richard Biener  changed:

   What|Removed |Added

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

--- Comment #21 from Richard Biener  ---
Martin, please work on this together with Honza (might be an actual duplicate).