> On 29 Sep 2025, at 16:29, Alfie Richards <[email protected]> wrote:
> 
> This looks good to me, but I cannot approve. 
> 
> Is it easy to add a test case? It would be great to have more coverage of FMV 
> in LTO to catch similar issues. 
> 

Yes. I might provide that case in a few days.

And there is a good news: After another patch to prevent virtual functions from 
auto generating FMV (one of my research project), I've successfully built 
SPECCPU (intrate, fprate) x (2006, 2017) including all hot functions to have 
FMV (even with Fortran benchmarks), without any issues. At least all hot 
functions in a generic CPU benchmark is being covered with LTO.

Thanks,
Yangyu Chen

> Thanks,
> Alfie
> 
> Sent from Outlook for iOS
> From: Yangyu Chen <[email protected]>
> Sent: Sunday, September 28, 2025 5:14:57 PM
> To: [email protected] <[email protected]>
> Cc: Richard Sandiford <[email protected]>; Alice Carlotti 
> <[email protected]>; Michael Meissner <[email protected]>; Alfie 
> Richards <[email protected]>; Jason Merrill <[email protected]>; Jeff Law 
> <[email protected]>; Kito Cheng <[email protected]>; David Edelsohn 
> <[email protected]>
> Subject: Re: [PATCH -fixes] fmv: Check DECL_VIRTUAL_P for virtual functions 
> for LTO handling   Ping.
> 
> On 31/7/2025 23:46, Yangyu Chen wrote:
> > When building with LTO, DECL_VINDEX is not always set for virtual
> > functions, which allows virtual functions to be incorrectly treated as
> > non-virtual and then being multi-versioned, and causes errors sometimes.
> > 
> > This patch addresses the issue by ensuring that we also check
> > DECL_VIRTUAL_P in addition to DECL_VINDEX when handling virtual
> > functions during multiversioning.
> > 
> > gcc/ChangeLog:
> > 
> >        * config/aarch64/aarch64.cc 
> > (aarch64_generate_version_dispatcher_body): Check DECL_VIRTUAL_P for 
> > virtual functions for LTO handling.
> >        * config/i386/i386-features.cc 
> > (ix86_generate_version_dispatcher_body): Ditto.
> >        * config/riscv/riscv.cc (riscv_generate_version_dispatcher_body): 
> > Ditto.
> >        * config/rs6000/rs6000.cc (rs6000_generate_version_dispatcher_body): 
> > Ditto.
> > 
> > Signed-off-by: Yangyu Chen <[email protected]>
> > ---
> > This should also be backported to GCC stable branches.
> > ---
> >   gcc/config/aarch64/aarch64.cc    | 2 +-
> >   gcc/config/i386/i386-features.cc | 2 +-
> >   gcc/config/riscv/riscv.cc        | 2 +-
> >   gcc/config/rs6000/rs6000.cc      | 2 +-
> >   4 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> > index fff8d9da49d..fd77e24ce12 100644
> > --- a/gcc/config/aarch64/aarch64.cc
> > +++ b/gcc/config/aarch64/aarch64.cc
> > @@ -20763,7 +20763,7 @@ aarch64_generate_version_dispatcher_body (void 
> > *node_p)
> >         not.  This happens for methods in derived classes that override
> >         virtual methods in base classes but are not explicitly marked as
> >         virtual.  */
> > -      if (DECL_VINDEX (versn->decl))
> > +      if (DECL_VIRTUAL_P (versn->decl) || DECL_VINDEX (versn->decl))
> >        sorry ("virtual function multiversioning not supported");
> >   
> >         fn_ver_vec.safe_push (versn->decl);
> > diff --git a/gcc/config/i386/i386-features.cc 
> > b/gcc/config/i386/i386-features.cc
> > index 31f3ee2ef17..043e86ece8e 100644
> > --- a/gcc/config/i386/i386-features.cc
> > +++ b/gcc/config/i386/i386-features.cc
> > @@ -4421,7 +4421,7 @@ ix86_generate_version_dispatcher_body (void *node_p)
> >         not.  This happens for methods in derived classes that override
> >         virtual methods in base classes but are not explicitly marked as
> >         virtual.  */
> > -      if (DECL_VINDEX (versn->decl))
> > +      if (DECL_VIRTUAL_P (versn->decl) || DECL_VINDEX (versn->decl))
> >        sorry ("virtual function multiversioning not supported");
> >   
> >         fn_ver_vec.safe_push (versn->decl);
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index a0657323f65..90f2cbce68e 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -13747,7 +13747,7 @@ riscv_generate_version_dispatcher_body (void 
> > *node_p)
> >         not.  This happens for methods in derived classes that override
> >         virtual methods in base classes but are not explicitly marked as
> >         virtual.  */
> > -      if (DECL_VINDEX (versn->decl))
> > +      if (DECL_VIRTUAL_P (versn->decl) || DECL_VINDEX (versn->decl))
> >        sorry ("virtual function multiversioning not supported");
> >   
> >         fn_ver_vec.safe_push (versn->decl);
> > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> > index 12dbde2bc63..0c13d524124 100644
> > --- a/gcc/config/rs6000/rs6000.cc
> > +++ b/gcc/config/rs6000/rs6000.cc
> > @@ -25656,7 +25656,7 @@ rs6000_generate_version_dispatcher_body (void 
> > *node_p)
> >         not.  This happens for methods in derived classes that override
> >         virtual methods in base classes but are not explicitly marked as
> >         virtual.  */
> > -      if (DECL_VINDEX (version->decl))
> > +      if (DECL_VIRTUAL_P (version->decl) || DECL_VINDEX (version->decl))
> >        sorry ("Virtual function multiversioning not supported");
> >   
> >         fn_ver_vec.safe_push (version->decl);
> 

Reply via email to