Re: [PATCH 0/1 V2] Target independent code for common infrastructure of load,store fusion for rs6000 and aarch64 target.

2024-02-15 Thread Ajit Agarwal 



On 15/02/24 10:43 pm, Alex Coplan wrote:
> So IIUC Richard was suggesting splitting into target-independent and
> target-dependent pieces within aarch64-ldp-fusion.cc as a first step,
> i.e. you introduce the abstractions (virtual functions) needed within
> that file.  That should hopefully be a relatively small diff.
> 
> Then in a separate patch you can move the target-independent parts out of
> config/aarch64.
> 
> Does that make sense?

Thanks a lot for explaining this. Sure I will do that and send the patch as per
above.

Thanks & Regards
Ajit

Re: [PATCH 0/1 V2] Target independent code for common infrastructure of load,store fusion for rs6000 and aarch64 target.

2024-02-15 Thread Alex Coplan 
On 15/02/2024 22:38, Ajit Agarwal wrote:
> Hello Alex:
> 
> On 15/02/24 10:12 pm, Alex Coplan wrote:
> > On 15/02/2024 21:24, Ajit Agarwal wrote:
> >> Hello Richard:
> >>
> >> As per your suggestion I have divided the patch into target independent
> >> and target dependent for aarch64 target. I kept aarch64-ldp-fusion same
> >> and did not change that.
> > 
> > I'm not sure this was what Richard suggested doing, though.
> > He said (from
> > https://gcc.gnu.org/pipermail/gcc-patches/2024-February/645545.html):
> > 
> >> Maybe one way of making the review easier would be to split the aarch64
> >> pass into the "target-dependent" and "target-independent" pieces
> >> in-place, i.e. keeping everything within aarch64-ldp-fusion.cc, and then
> >> (as separate patches) move the target-independent pieces outside
> >> config/aarch64.
> > 
> > but this adds the target-independent parts separately instead of
> > splitting it out within config/aarch64 (which I agree should make the
> > review easier).
> 
> I am sorry I didnt follow. Can you kindly elaborate on this.

So IIUC Richard was suggesting splitting into target-independent and
target-dependent pieces within aarch64-ldp-fusion.cc as a first step,
i.e. you introduce the abstractions (virtual functions) needed within
that file.  That should hopefully be a relatively small diff.

Then in a separate patch you can move the target-independent parts out of
config/aarch64.

Does that make sense?

Thanks,
Alex

> 
> Thanks & Regards
> Ajit
> > 
> > Thanks,
> > Alex
> > 
> >>
> >> Common infrastructure of load store pair fusion is divided into
> >> target independent and target dependent code for rs6000 and aarch64
> >> target.
> >>
> >> Target independent code is structured in the following files.
> >> gcc/pair-fusion-base.h
> >> gcc/pair-fusion-common.cc
> >> gcc/pair-fusion.cc
> >>
> >> Target independent code is the Generic code with pure virtual
> >> function to interface betwwen target independent and dependent
> >> code.
> >>
> >> Thanks & Regards
> >> Ajit
> >>
> >> Target independent code for common infrastructure of load
> >> store fusion for rs6000 and aarch64 target.
> >>
> >> Common infrastructure of load store pair fusion is divided into
> >> target independent and target dependent code for rs6000 and aarch64
> >> target.
> >>
> >> Target independent code is structured in the following files.
> >> gcc/pair-fusion-base.h
> >> gcc/pair-fusion-common.cc
> >> gcc/pair-fusion.cc
> >>
> >> Target independent code is the Generic code with pure virtual
> >> function to interface betwwen target independent and dependent
> >> code.
> >>
> >> 2024-02-15  Ajit Kumar Agarwal  
> >>
> >> gcc/ChangeLog:
> >>
> >>* pair-fusion-base.h: Generic header code for load store fusion
> >>that can be shared across different architectures.
> >>* pair-fusion-common.cc: Generic source code for load store
> >>fusion that can be shared across different architectures.
> >>* pair-fusion.cc: Generic implementation of pair_fusion class
> >>defined in pair-fusion-base.h
> >>* Makefile.in: Add new executable pair-fusion.o and
> >>pair-fusion-common.o.
> >> ---
> >>  gcc/Makefile.in   |2 +
> >>  gcc/pair-fusion-base.h|  586 ++
> >>  gcc/pair-fusion-common.cc | 1202 
> >>  gcc/pair-fusion.cc| 1225 +
> >>  4 files changed, 3015 insertions(+)
> >>  create mode 100644 gcc/pair-fusion-base.h
> >>  create mode 100644 gcc/pair-fusion-common.cc
> >>  create mode 100644 gcc/pair-fusion.cc
> >>
> >> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> >> index a74761b7ab3..df5061ddfe7 100644
> >> --- a/gcc/Makefile.in
> >> +++ b/gcc/Makefile.in
> >> @@ -1563,6 +1563,8 @@ OBJS = \
> >>ipa-strub.o \
> >>ipa.o \
> >>ira.o \
> >> +  pair-fusion-common.o \
> >> +  pair-fusion.o \
> >>ira-build.o \
> >>ira-costs.o \
> >>ira-conflicts.o \
> >> diff --git a/gcc/pair-fusion-base.h b/gcc/pair-fusion-base.h
> >> new file mode 100644
> >> index 000..fdaf4fd743d
> >> --- /dev/null
> >> +++ b/gcc/pair-fusion-base.h
> >> @@ -0,0 +1,586 @@
> >> +// Generic code for Pair MEM  fusion optimization pass.
> >> +// Copyright (C) 2023-2024 Free Software Foundation, Inc.
> >> +//
> >> +// This file is part of GCC.
> >> +//
> >> +// GCC is free software; you can redistribute it and/or modify it
> >> +// under the terms of the GNU General Public License as published by
> >> +// the Free Software Foundation; either version 3, or (at your option)
> >> +// any later version.
> >> +//
> >> +// GCC is distributed in the hope that it will be useful, but
> >> +// WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> +// General Public License for more details.
> >> +//
> >> +// You should have received a copy of the GNU General Public License
> >> +// along with GCC; see the file

Re: [PATCH 0/1 V2] Target independent code for common infrastructure of load,store fusion for rs6000 and aarch64 target.

2024-02-15 Thread Ajit Agarwal 
Hello Alex:

On 15/02/24 10:12 pm, Alex Coplan wrote:
> On 15/02/2024 21:24, Ajit Agarwal wrote:
>> Hello Richard:
>>
>> As per your suggestion I have divided the patch into target independent
>> and target dependent for aarch64 target. I kept aarch64-ldp-fusion same
>> and did not change that.
> 
> I'm not sure this was what Richard suggested doing, though.
> He said (from
> https://gcc.gnu.org/pipermail/gcc-patches/2024-February/645545.html):
> 
>> Maybe one way of making the review easier would be to split the aarch64
>> pass into the "target-dependent" and "target-independent" pieces
>> in-place, i.e. keeping everything within aarch64-ldp-fusion.cc, and then
>> (as separate patches) move the target-independent pieces outside
>> config/aarch64.
> 
> but this adds the target-independent parts separately instead of
> splitting it out within config/aarch64 (which I agree should make the
> review easier).

I am sorry I didnt follow. Can you kindly elaborate on this.

Thanks & Regards
Ajit
> 
> Thanks,
> Alex
> 
>>
>> Common infrastructure of load store pair fusion is divided into
>> target independent and target dependent code for rs6000 and aarch64
>> target.
>>
>> Target independent code is structured in the following files.
>> gcc/pair-fusion-base.h
>> gcc/pair-fusion-common.cc
>> gcc/pair-fusion.cc
>>
>> Target independent code is the Generic code with pure virtual
>> function to interface betwwen target independent and dependent
>> code.
>>
>> Thanks & Regards
>> Ajit
>>
>> Target independent code for common infrastructure of load
>> store fusion for rs6000 and aarch64 target.
>>
>> Common infrastructure of load store pair fusion is divided into
>> target independent and target dependent code for rs6000 and aarch64
>> target.
>>
>> Target independent code is structured in the following files.
>> gcc/pair-fusion-base.h
>> gcc/pair-fusion-common.cc
>> gcc/pair-fusion.cc
>>
>> Target independent code is the Generic code with pure virtual
>> function to interface betwwen target independent and dependent
>> code.
>>
>> 2024-02-15  Ajit Kumar Agarwal  
>>
>> gcc/ChangeLog:
>>
>>  * pair-fusion-base.h: Generic header code for load store fusion
>>  that can be shared across different architectures.
>>  * pair-fusion-common.cc: Generic source code for load store
>>  fusion that can be shared across different architectures.
>>  * pair-fusion.cc: Generic implementation of pair_fusion class
>>  defined in pair-fusion-base.h
>>  * Makefile.in: Add new executable pair-fusion.o and
>>  pair-fusion-common.o.
>> ---
>>  gcc/Makefile.in   |2 +
>>  gcc/pair-fusion-base.h|  586 ++
>>  gcc/pair-fusion-common.cc | 1202 
>>  gcc/pair-fusion.cc| 1225 +
>>  4 files changed, 3015 insertions(+)
>>  create mode 100644 gcc/pair-fusion-base.h
>>  create mode 100644 gcc/pair-fusion-common.cc
>>  create mode 100644 gcc/pair-fusion.cc
>>
>> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
>> index a74761b7ab3..df5061ddfe7 100644
>> --- a/gcc/Makefile.in
>> +++ b/gcc/Makefile.in
>> @@ -1563,6 +1563,8 @@ OBJS = \
>>  ipa-strub.o \
>>  ipa.o \
>>  ira.o \
>> +pair-fusion-common.o \
>> +pair-fusion.o \
>>  ira-build.o \
>>  ira-costs.o \
>>  ira-conflicts.o \
>> diff --git a/gcc/pair-fusion-base.h b/gcc/pair-fusion-base.h
>> new file mode 100644
>> index 000..fdaf4fd743d
>> --- /dev/null
>> +++ b/gcc/pair-fusion-base.h
>> @@ -0,0 +1,586 @@
>> +// Generic code for Pair MEM  fusion optimization pass.
>> +// Copyright (C) 2023-2024 Free Software Foundation, Inc.
>> +//
>> +// This file is part of GCC.
>> +//
>> +// GCC is free software; you can redistribute it and/or modify it
>> +// under the terms of the GNU General Public License as published by
>> +// the Free Software Foundation; either version 3, or (at your option)
>> +// any later version.
>> +//
>> +// GCC is distributed in the hope that it will be useful, but
>> +// WITHOUT ANY WARRANTY; without even the implied warranty of
>> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +// General Public License for more details.
>> +//
>> +// You should have received a copy of the GNU General Public License
>> +// along with GCC; see the file COPYING3.  If not see
>> +// .
>> +
>> +#ifndef GCC_PAIR_FUSION_H
>> +#define GCC_PAIR_FUSION_H
>> +#define INCLUDE_ALGORITHM
>> +#define INCLUDE_FUNCTIONAL
>> +#define INCLUDE_LIST
>> +#define INCLUDE_TYPE_TRAITS
>> +#include "config.h"
>> +#include "system.h"
>> +#include "coretypes.h"
>> +#include "backend.h"
>> +#include "rtl.h"
>> +#include "df.h"
>> +#include "rtl-iter.h"
>> +#include "rtl-ssa.h"
>> +#include "cfgcleanup.h"
>> +#include "tree-pass.h"
>> +#include "ordered-hash-map.h"
>> +#include "tree-dfa.h"
>> +#include "fold-const.h"
>> +#include "tree-hash-traits.h"
>> +#include "print-tree.h"
>> +#include

Re: [PATCH 0/1 V2] Target independent code for common infrastructure of load,store fusion for rs6000 and aarch64 target.

2024-02-15 Thread Alex Coplan 
On 15/02/2024 21:24, Ajit Agarwal wrote:
> Hello Richard:
> 
> As per your suggestion I have divided the patch into target independent
> and target dependent for aarch64 target. I kept aarch64-ldp-fusion same
> and did not change that.

I'm not sure this was what Richard suggested doing, though.
He said (from
https://gcc.gnu.org/pipermail/gcc-patches/2024-February/645545.html):

> Maybe one way of making the review easier would be to split the aarch64
> pass into the "target-dependent" and "target-independent" pieces
> in-place, i.e. keeping everything within aarch64-ldp-fusion.cc, and then
> (as separate patches) move the target-independent pieces outside
> config/aarch64.

but this adds the target-independent parts separately instead of
splitting it out within config/aarch64 (which I agree should make the
review easier).

Thanks,
Alex

> 
> Common infrastructure of load store pair fusion is divided into
> target independent and target dependent code for rs6000 and aarch64
> target.
> 
> Target independent code is structured in the following files.
> gcc/pair-fusion-base.h
> gcc/pair-fusion-common.cc
> gcc/pair-fusion.cc
> 
> Target independent code is the Generic code with pure virtual
> function to interface betwwen target independent and dependent
> code.
> 
> Thanks & Regards
> Ajit
> 
> Target independent code for common infrastructure of load
> store fusion for rs6000 and aarch64 target.
> 
> Common infrastructure of load store pair fusion is divided into
> target independent and target dependent code for rs6000 and aarch64
> target.
> 
> Target independent code is structured in the following files.
> gcc/pair-fusion-base.h
> gcc/pair-fusion-common.cc
> gcc/pair-fusion.cc
> 
> Target independent code is the Generic code with pure virtual
> function to interface betwwen target independent and dependent
> code.
> 
> 2024-02-15  Ajit Kumar Agarwal  
> 
> gcc/ChangeLog:
> 
>   * pair-fusion-base.h: Generic header code for load store fusion
>   that can be shared across different architectures.
>   * pair-fusion-common.cc: Generic source code for load store
>   fusion that can be shared across different architectures.
>   * pair-fusion.cc: Generic implementation of pair_fusion class
>   defined in pair-fusion-base.h
>   * Makefile.in: Add new executable pair-fusion.o and
>   pair-fusion-common.o.
> ---
>  gcc/Makefile.in   |2 +
>  gcc/pair-fusion-base.h|  586 ++
>  gcc/pair-fusion-common.cc | 1202 
>  gcc/pair-fusion.cc| 1225 +
>  4 files changed, 3015 insertions(+)
>  create mode 100644 gcc/pair-fusion-base.h
>  create mode 100644 gcc/pair-fusion-common.cc
>  create mode 100644 gcc/pair-fusion.cc
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index a74761b7ab3..df5061ddfe7 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1563,6 +1563,8 @@ OBJS = \
>   ipa-strub.o \
>   ipa.o \
>   ira.o \
> + pair-fusion-common.o \
> + pair-fusion.o \
>   ira-build.o \
>   ira-costs.o \
>   ira-conflicts.o \
> diff --git a/gcc/pair-fusion-base.h b/gcc/pair-fusion-base.h
> new file mode 100644
> index 000..fdaf4fd743d
> --- /dev/null
> +++ b/gcc/pair-fusion-base.h
> @@ -0,0 +1,586 @@
> +// Generic code for Pair MEM  fusion optimization pass.
> +// Copyright (C) 2023-2024 Free Software Foundation, Inc.
> +//
> +// This file is part of GCC.
> +//
> +// GCC is free software; you can redistribute it and/or modify it
> +// under the terms of the GNU General Public License as published by
> +// the Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +//
> +// GCC is distributed in the hope that it will be useful, but
> +// WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +// General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public License
> +// along with GCC; see the file COPYING3.  If not see
> +// .
> +
> +#ifndef GCC_PAIR_FUSION_H
> +#define GCC_PAIR_FUSION_H
> +#define INCLUDE_ALGORITHM
> +#define INCLUDE_FUNCTIONAL
> +#define INCLUDE_LIST
> +#define INCLUDE_TYPE_TRAITS
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "backend.h"
> +#include "rtl.h"
> +#include "df.h"
> +#include "rtl-iter.h"
> +#include "rtl-ssa.h"
> +#include "cfgcleanup.h"
> +#include "tree-pass.h"
> +#include "ordered-hash-map.h"
> +#include "tree-dfa.h"
> +#include "fold-const.h"
> +#include "tree-hash-traits.h"
> +#include "print-tree.h"
> +#include "insn-attr.h"
> +using namespace rtl_ssa;
> +// We pack these fields (load_p, fpsimd_p, and size) into an integer
> +// (LFS) which we use as part of the key into the main hash tables.
> +//
> +// The idea is that we group candidates together only if they agree on
>