Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-15 Thread Jan Hubicka
> > + if (POINTER_TYPE_P (TREE_TYPE (t1))) > > +{ > > + if (SSA_NAME_PTR_INFO (t1)) > > + { > > + if (!SSA_NAME_PTR_INFO (t2) > > + || SSA_NAME_PTR_INFO (t1)->align != SSA_NAME_PTR_INFO (t2)->align > > + || SSA_NAME_PTR_INFO (t1)->misalign != SSA_NAME_PTR_INFO > >

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-15 Thread Richard Biener
On Thu, 14 Mar 2024, Jan Hubicka wrote: > > > We have wrong code with LTO, too. > > > > I know. > > > > > The problem is that IPA passes (and > > > not only that, loop analysis too) does analysis at compile time (with > > > value numbers in) and streams the info separately. > > > > And that is

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-14 Thread Jan Hubicka
> > Otherwise > > I will add your testcase for this patch and commit this one. > > Statistically we almost never merge functions with different value > > ranges (three in testsuite, 0 during bootstrap, 1 during LTO bootstrap > > and probably few in LLVM build - there are 15 cases reported, but

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-14 Thread Jakub Jelinek
On Thu, Mar 14, 2024 at 05:16:59PM +0100, Jan Hubicka wrote: > Sorry, this was bit of a misunderstanding: I tought you still considered > the original patch to be full fix, while I tought I should look into it > more and dig out more issues. This is bit of can of worms. Overall I > think the

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-14 Thread Jan Hubicka
> > We have wrong code with LTO, too. > > I know. > > > The problem is that IPA passes (and > > not only that, loop analysis too) does analysis at compile time (with > > value numbers in) and streams the info separately. > > And that is desirable, because otherwise it simply couldn't derive any

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-14 Thread Jan Hubicka
> > int test (int a) > > { > > return a>0 ? CST1: CST2; > > } > > > > gets same hash value no matter what CST1/CST2 is. I added hasher and I > > am re-running stats. > > The hash should be commutative here at least. It needs to match what comparator is doing later, and sadly it does not try

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-13 Thread Jakub Jelinek
On Wed, Mar 13, 2024 at 12:18:45PM +0100, Jan Hubicka wrote: > > On Wed, Mar 13, 2024 at 10:55:07AM +0100, Jan Hubicka wrote: > > > > > So the ipa_jump_func are I think the only thing that actually can > > > > > differ > > > > > on the ICF merging candidates from value range POV. > > > > > > > >

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-13 Thread Jan Hubicka
> On Wed, Mar 13, 2024 at 10:55:07AM +0100, Jan Hubicka wrote: > > > > So the ipa_jump_func are I think the only thing that actually can differ > > > > on the ICF merging candidates from value range POV. > > > > > > I agree. Btw, I would have approved the original patch in this > > > thread that

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-13 Thread Jakub Jelinek
On Wed, Mar 13, 2024 at 10:55:07AM +0100, Jan Hubicka wrote: > > > So the ipa_jump_func are I think the only thing that actually can differ > > > on the ICF merging candidates from value range POV. > > > > I agree. Btw, I would have approved the original patch in this > > thread that wipes

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-13 Thread Richard Biener
On Wed, 13 Mar 2024, Jan Hubicka wrote: > > On Tue, 12 Mar 2024, Jakub Jelinek wrote: > > > > > On Tue, Mar 12, 2024 at 05:21:58PM +0100, Jakub Jelinek wrote: > > > > On Tue, Mar 12, 2024 at 10:46:42AM +0100, Jan Hubicka wrote: > > > > > I am sorry for delaying this. I made the variant that

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-13 Thread Jan Hubicka
> On Tue, 12 Mar 2024, Jakub Jelinek wrote: > > > On Tue, Mar 12, 2024 at 05:21:58PM +0100, Jakub Jelinek wrote: > > > On Tue, Mar 12, 2024 at 10:46:42AM +0100, Jan Hubicka wrote: > > > > I am sorry for delaying this. I made the variant that simply compares > > > > value range of functions and

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-13 Thread Richard Biener
On Tue, 12 Mar 2024, Jakub Jelinek wrote: > On Tue, Mar 12, 2024 at 05:21:58PM +0100, Jakub Jelinek wrote: > > On Tue, Mar 12, 2024 at 10:46:42AM +0100, Jan Hubicka wrote: > > > I am sorry for delaying this. I made the variant that simply compares > > > value range of functions and prevents

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-12 Thread Jakub Jelinek
On Tue, Mar 12, 2024 at 05:21:58PM +0100, Jakub Jelinek wrote: > On Tue, Mar 12, 2024 at 10:46:42AM +0100, Jan Hubicka wrote: > > I am sorry for delaying this. I made the variant that simply compares > > value range of functions and prevents merging if they diverge and wanted > > to make some

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-12 Thread Jakub Jelinek
On Tue, Mar 12, 2024 at 10:46:42AM +0100, Jan Hubicka wrote: > I am sorry for delaying this. I made the variant that simply compares > value range of functions and prevents merging if they diverge and wanted > to make some bigger statistics. This made me notice some performance > problems on

Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-12 Thread Jan Hubicka
> Hi! Hi, > > On Thu, Feb 15, 2024 at 08:29:24AM +0100, Jakub Jelinek wrote: > > 2024-02-15 Jakub Jelinek > > > > PR middle-end/113907 > > * ipa-icf.cc (sem_item_optimizer::merge_classes): Reset > > SSA_NAME_RANGE_INFO and SSA_NAME_PTR_INFO on successfully ICF merged > >

Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907]

2024-03-12 Thread Jakub Jelinek
Hi! On Thu, Feb 15, 2024 at 08:29:24AM +0100, Jakub Jelinek wrote: > 2024-02-15 Jakub Jelinek > > PR middle-end/113907 > * ipa-icf.cc (sem_item_optimizer::merge_classes): Reset > SSA_NAME_RANGE_INFO and SSA_NAME_PTR_INFO on successfully ICF merged > functions. > >