Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Jeff Law via Gcc-patches]

On 11/14/22 08:38, Christoph Müllner wrote:


Ok, I will add the check.

Still, I don't think that the optimization changes the behavior in the
case of different ABIs
(i.e. the situation is problematic regardless if there is an indirect call
or a guarded direct call).
What could be done additionally to dropping the candidate is to emit a
warning (similar like ipa-devirt does for ODR violations).


The one target I'm aware of where this is most likely to cause problems 
would be the older 32bit PA SOM ABI.  It has the property that the ABI 
uses different registers for parameter passing for direct vs indirect 
calls.  But with all this happening before RTL generation we should be 
fine, even for that ABI (and its generally safe in that ABI to change an 
indirect to a direct call, but not vice-versa).



Jeff

Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Christoph Müllner]

On Mon, Nov 14, 2022 at 2:48 PM Richard Biener 
wrote:

> On Mon, Nov 14, 2022 at 12:46 PM Christoph Müllner
>  wrote:
> >
> >
> >
> > On Mon, Nov 14, 2022 at 11:10 AM Richard Biener <
> richard.guent...@gmail.com> wrote:
> >>
> >> On Mon, Nov 14, 2022 at 10:32 AM Christoph Müllner
> >>  wrote:
> >> >
> >> >
> >> >
> >> > On Mon, Nov 14, 2022 at 10:00 AM Richard Biener <
> richard.guent...@gmail.com> wrote:
> >> >>
> >> >> On Mon, Nov 14, 2022 at 9:13 AM Christoph Müllner
> >> >>  wrote:
> >> >> >
> >> >> >
> >> >> >
> >> >> > On Mon, Nov 14, 2022 at 8:31 AM Richard Biener <
> richard.guent...@gmail.com> wrote:
> >> >> >>
> >> >> >> On Sun, Nov 13, 2022 at 4:09 PM Christoph Muellner
> >> >> >>  wrote:
> >> >> >> >
> >> >> >> > From: Christoph Müllner 
> >> >> >> >
> >> >> >> > This patch adds a new pass that looks up function pointer
> assignments,
> >> >> >> > and adds guarded direct calls to the call sites of the function
> >> >> >> > pointers.
> >> >> >> >
> >> >> >> > E.g.: Lets assume an assignment to a function pointer as
> follows:
> >> >> >> > b->cb = 
> >> >> >> >   Other part of the program can use the function pointer as
> follows:
> >> >> >> > b->cb ();
> >> >> >> >   With this pass the invocation will be transformed to:
> >> >> >> > if (b->cb == myfun)
> >> >> >> >   myfun();
> >> >> >> > else
> >> >> >> >b->cb ()
> >> >> >> >
> >> >> >> > The impact of the dynamic guard is expected to be less than the
> speedup
> >> >> >> > gained by enabled optimizations (e.g. inlining or constant
> propagation).
> >> >> >>
> >> >> >> We have speculative devirtualization doing this very transform,
> shouldn't you
> >> >> >> instead improve that instead of inventing another specialized
> pass?
> >> >> >
> >> >> >
> >> >> > Yes, it can be integrated into ipa-devirt.
> >> >> >
> >> >> > The reason we initially decided to move it into its own file was
> that C++ devirtualization
> >> >> > and function pointer dereferencing/devirtualization will likely
> not use the same analysis.
> >> >> > E.g. ODR only applies to C++, C++ tables are not directly exposed
> to the user.
> >> >> > So we figured that different things should not be merged together,
> but a reuse
> >> >> > of common code to avoid duplication is mandatory.
> >> >>
> >> >> Btw, in other context the idea came up to build candidates based on
> available
> >> >> API/ABI (that can be indirectly called).  That would help for
> example the
> >> >> get_ref calls in refine_subpel in the x264 benchmark.  Maybe what you
> >> >> do is actually
> >> >> the very same thing (but look for explicit address-taking) - I didn't
> >> >> look into whether
> >> >> you prune the list of candidates based on API/ABI.
> >> >
> >> >
> >> > No, I don't consider API/ABI at all (do you have a pointer so I can
> get a better understanding of that idea?).
> >>
> >> No, it was just an idea discussed internally.
> >>
> >> > Adding guards for all possible functions with the same API/ABI seems
> expensive (I might misunderstand the idea).
> >> > My patch adds a maximum of 1 test per call site.
> >> >
> >> > What I do is looking which addresses are assigned to the function
> pointer.
> >> > If there is more than one assigned function, I drop the function
> pointer from the list of candidates.
> >>
> >> OK.  If the program is type correct that's probably going to work well
> >> enough.  If there are more than
> >> one candidates then you could prune those by simple API checks, like
> >> match up the number of arguments
> >> or void vs. non-void return type.  More advanced pruning might lose
> >> some valid candidates (API vs.
> >> ABI compatibility), but it's only heuristic pruning in any case.
> >>
> >> It would probably help depending on what exactly "assigned to the
> >> function pointer" means.  If the
> >> function pointer is not from directly visible static storage then
> >> matching up assignments and uses
> >> is going to be a difficult IPA problem itself.  So our original idea
> was for
> >>
> >>  (*fnptr) (args ...);
> >>
> >> look for all possible definitions in the (LTO) unit that match the
> >> call signature and that have their
> >> address taken and that possibly could be pointed to by fnptr and if
> >> that's a single one, speculatively
> >> devirtualize that.
> >
> >
> > Understood. That's an interesting idea.
> > Assuming that functions with identical signatures are rare,
> > both approaches should find similar candidates.
> >
> > I wonder why the API/ABI compatibility checks are needed
> > if we only consider functions assigned to a function pointer.
> > I.e. if call-site and callee don't match, wouldn't the indirect call
> > suffer from the same incompatibility?
>
> At least in C land mismatches are not unheard of (working across TUs).
>
> >
> > The patch currently looks at the following properties of the RHS of a
> function pointer assignment:
> > * rhs = gimple_assign_rhs1 (stmt)
> > * rhs_t = TREE_TYPE 

Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Richard Biener via Gcc-patches]

On Mon, Nov 14, 2022 at 12:46 PM Christoph Müllner
 wrote:
>
>
>
> On Mon, Nov 14, 2022 at 11:10 AM Richard Biener  
> wrote:
>>
>> On Mon, Nov 14, 2022 at 10:32 AM Christoph Müllner
>>  wrote:
>> >
>> >
>> >
>> > On Mon, Nov 14, 2022 at 10:00 AM Richard Biener 
>> >  wrote:
>> >>
>> >> On Mon, Nov 14, 2022 at 9:13 AM Christoph Müllner
>> >>  wrote:
>> >> >
>> >> >
>> >> >
>> >> > On Mon, Nov 14, 2022 at 8:31 AM Richard Biener 
>> >> >  wrote:
>> >> >>
>> >> >> On Sun, Nov 13, 2022 at 4:09 PM Christoph Muellner
>> >> >>  wrote:
>> >> >> >
>> >> >> > From: Christoph Müllner 
>> >> >> >
>> >> >> > This patch adds a new pass that looks up function pointer 
>> >> >> > assignments,
>> >> >> > and adds guarded direct calls to the call sites of the function
>> >> >> > pointers.
>> >> >> >
>> >> >> > E.g.: Lets assume an assignment to a function pointer as follows:
>> >> >> > b->cb = 
>> >> >> >   Other part of the program can use the function pointer as 
>> >> >> > follows:
>> >> >> > b->cb ();
>> >> >> >   With this pass the invocation will be transformed to:
>> >> >> > if (b->cb == myfun)
>> >> >> >   myfun();
>> >> >> > else
>> >> >> >b->cb ()
>> >> >> >
>> >> >> > The impact of the dynamic guard is expected to be less than the 
>> >> >> > speedup
>> >> >> > gained by enabled optimizations (e.g. inlining or constant 
>> >> >> > propagation).
>> >> >>
>> >> >> We have speculative devirtualization doing this very transform, 
>> >> >> shouldn't you
>> >> >> instead improve that instead of inventing another specialized pass?
>> >> >
>> >> >
>> >> > Yes, it can be integrated into ipa-devirt.
>> >> >
>> >> > The reason we initially decided to move it into its own file was that 
>> >> > C++ devirtualization
>> >> > and function pointer dereferencing/devirtualization will likely not use 
>> >> > the same analysis.
>> >> > E.g. ODR only applies to C++, C++ tables are not directly exposed to 
>> >> > the user.
>> >> > So we figured that different things should not be merged together, but 
>> >> > a reuse
>> >> > of common code to avoid duplication is mandatory.
>> >>
>> >> Btw, in other context the idea came up to build candidates based on 
>> >> available
>> >> API/ABI (that can be indirectly called).  That would help for example the
>> >> get_ref calls in refine_subpel in the x264 benchmark.  Maybe what you
>> >> do is actually
>> >> the very same thing (but look for explicit address-taking) - I didn't
>> >> look into whether
>> >> you prune the list of candidates based on API/ABI.
>> >
>> >
>> > No, I don't consider API/ABI at all (do you have a pointer so I can get a 
>> > better understanding of that idea?).
>>
>> No, it was just an idea discussed internally.
>>
>> > Adding guards for all possible functions with the same API/ABI seems 
>> > expensive (I might misunderstand the idea).
>> > My patch adds a maximum of 1 test per call site.
>> >
>> > What I do is looking which addresses are assigned to the function pointer.
>> > If there is more than one assigned function, I drop the function pointer 
>> > from the list of candidates.
>>
>> OK.  If the program is type correct that's probably going to work well
>> enough.  If there are more than
>> one candidates then you could prune those by simple API checks, like
>> match up the number of arguments
>> or void vs. non-void return type.  More advanced pruning might lose
>> some valid candidates (API vs.
>> ABI compatibility), but it's only heuristic pruning in any case.
>>
>> It would probably help depending on what exactly "assigned to the
>> function pointer" means.  If the
>> function pointer is not from directly visible static storage then
>> matching up assignments and uses
>> is going to be a difficult IPA problem itself.  So our original idea was for
>>
>>  (*fnptr) (args ...);
>>
>> look for all possible definitions in the (LTO) unit that match the
>> call signature and that have their
>> address taken and that possibly could be pointed to by fnptr and if
>> that's a single one, speculatively
>> devirtualize that.
>
>
> Understood. That's an interesting idea.
> Assuming that functions with identical signatures are rare,
> both approaches should find similar candidates.
>
> I wonder why the API/ABI compatibility checks are needed
> if we only consider functions assigned to a function pointer.
> I.e. if call-site and callee don't match, wouldn't the indirect call
> suffer from the same incompatibility?

At least in C land mismatches are not unheard of (working across TUs).

>
> The patch currently looks at the following properties of the RHS of a 
> function pointer assignment:
> * rhs = gimple_assign_rhs1 (stmt)
> * rhs_t = TREE_TYPE (rhs)
> * possible_decl = TREE_OPERAND (rhs, 0)
> * node = cgraph_node::get (possible_decl)
>
> And the following rules are currently enforced:
> * TREE_CODE (rhs) == ADDR_EXPR
> * TREE_CODE (rhs_t) == POINTER_TYPE
> * TREE_CODE (TREE_TYPE (rhs_t)) == FUNCTION_TYPE
> * 

Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Christoph Müllner]

On Mon, Nov 14, 2022 at 11:10 AM Richard Biener 
wrote:

> On Mon, Nov 14, 2022 at 10:32 AM Christoph Müllner
>  wrote:
> >
> >
> >
> > On Mon, Nov 14, 2022 at 10:00 AM Richard Biener <
> richard.guent...@gmail.com> wrote:
> >>
> >> On Mon, Nov 14, 2022 at 9:13 AM Christoph Müllner
> >>  wrote:
> >> >
> >> >
> >> >
> >> > On Mon, Nov 14, 2022 at 8:31 AM Richard Biener <
> richard.guent...@gmail.com> wrote:
> >> >>
> >> >> On Sun, Nov 13, 2022 at 4:09 PM Christoph Muellner
> >> >>  wrote:
> >> >> >
> >> >> > From: Christoph Müllner 
> >> >> >
> >> >> > This patch adds a new pass that looks up function pointer
> assignments,
> >> >> > and adds guarded direct calls to the call sites of the function
> >> >> > pointers.
> >> >> >
> >> >> > E.g.: Lets assume an assignment to a function pointer as follows:
> >> >> > b->cb = 
> >> >> >   Other part of the program can use the function pointer as
> follows:
> >> >> > b->cb ();
> >> >> >   With this pass the invocation will be transformed to:
> >> >> > if (b->cb == myfun)
> >> >> >   myfun();
> >> >> > else
> >> >> >b->cb ()
> >> >> >
> >> >> > The impact of the dynamic guard is expected to be less than the
> speedup
> >> >> > gained by enabled optimizations (e.g. inlining or constant
> propagation).
> >> >>
> >> >> We have speculative devirtualization doing this very transform,
> shouldn't you
> >> >> instead improve that instead of inventing another specialized pass?
> >> >
> >> >
> >> > Yes, it can be integrated into ipa-devirt.
> >> >
> >> > The reason we initially decided to move it into its own file was that
> C++ devirtualization
> >> > and function pointer dereferencing/devirtualization will likely not
> use the same analysis.
> >> > E.g. ODR only applies to C++, C++ tables are not directly exposed to
> the user.
> >> > So we figured that different things should not be merged together,
> but a reuse
> >> > of common code to avoid duplication is mandatory.
> >>
> >> Btw, in other context the idea came up to build candidates based on
> available
> >> API/ABI (that can be indirectly called).  That would help for example
> the
> >> get_ref calls in refine_subpel in the x264 benchmark.  Maybe what you
> >> do is actually
> >> the very same thing (but look for explicit address-taking) - I didn't
> >> look into whether
> >> you prune the list of candidates based on API/ABI.
> >
> >
> > No, I don't consider API/ABI at all (do you have a pointer so I can get
> a better understanding of that idea?).
>
> No, it was just an idea discussed internally.
>
> > Adding guards for all possible functions with the same API/ABI seems
> expensive (I might misunderstand the idea).
> > My patch adds a maximum of 1 test per call site.
> >
> > What I do is looking which addresses are assigned to the function
> pointer.
> > If there is more than one assigned function, I drop the function pointer
> from the list of candidates.
>
> OK.  If the program is type correct that's probably going to work well
> enough.  If there are more than
> one candidates then you could prune those by simple API checks, like
> match up the number of arguments
> or void vs. non-void return type.  More advanced pruning might lose
> some valid candidates (API vs.
> ABI compatibility), but it's only heuristic pruning in any case.
>
> It would probably help depending on what exactly "assigned to the
> function pointer" means.  If the
> function pointer is not from directly visible static storage then
> matching up assignments and uses
> is going to be a difficult IPA problem itself.  So our original idea was
> for
>
>  (*fnptr) (args ...);
>
> look for all possible definitions in the (LTO) unit that match the
> call signature and that have their
> address taken and that possibly could be pointed to by fnptr and if
> that's a single one, speculatively
> devirtualize that.
>

Understood. That's an interesting idea.
Assuming that functions with identical signatures are rare,
both approaches should find similar candidates.

I wonder why the API/ABI compatibility checks are needed
if we only consider functions assigned to a function pointer.
I.e. if call-site and callee don't match, wouldn't the indirect call
suffer from the same incompatibility?

The patch currently looks at the following properties of the RHS of a
function pointer assignment:
* rhs = gimple_assign_rhs1 (stmt)
* rhs_t = TREE_TYPE (rhs)
* possible_decl = TREE_OPERAND (rhs, 0)
* node = cgraph_node::get (possible_decl)

And the following rules are currently enforced:
* TREE_CODE (rhs) == ADDR_EXPR
* TREE_CODE (rhs_t) == POINTER_TYPE
* TREE_CODE (TREE_TYPE (rhs_t)) == FUNCTION_TYPE
* TREE_CODE (possible_decl) == FUNCTION_DECL



> > I just checked in the dump file, and the patch also dereferences the
> indirect calls to get_ref in refine_subpel.
>
> IIRC the x264 case has a global variable with all the function
> pointers so your implementation
> will likely pick up the single assignment to the 

Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Richard Biener via Gcc-patches]

On Mon, Nov 14, 2022 at 10:32 AM Christoph Müllner
 wrote:
>
>
>
> On Mon, Nov 14, 2022 at 10:00 AM Richard Biener  
> wrote:
>>
>> On Mon, Nov 14, 2022 at 9:13 AM Christoph Müllner
>>  wrote:
>> >
>> >
>> >
>> > On Mon, Nov 14, 2022 at 8:31 AM Richard Biener 
>> >  wrote:
>> >>
>> >> On Sun, Nov 13, 2022 at 4:09 PM Christoph Muellner
>> >>  wrote:
>> >> >
>> >> > From: Christoph Müllner 
>> >> >
>> >> > This patch adds a new pass that looks up function pointer assignments,
>> >> > and adds guarded direct calls to the call sites of the function
>> >> > pointers.
>> >> >
>> >> > E.g.: Lets assume an assignment to a function pointer as follows:
>> >> > b->cb = 
>> >> >   Other part of the program can use the function pointer as follows:
>> >> > b->cb ();
>> >> >   With this pass the invocation will be transformed to:
>> >> > if (b->cb == myfun)
>> >> >   myfun();
>> >> > else
>> >> >b->cb ()
>> >> >
>> >> > The impact of the dynamic guard is expected to be less than the speedup
>> >> > gained by enabled optimizations (e.g. inlining or constant propagation).
>> >>
>> >> We have speculative devirtualization doing this very transform, shouldn't 
>> >> you
>> >> instead improve that instead of inventing another specialized pass?
>> >
>> >
>> > Yes, it can be integrated into ipa-devirt.
>> >
>> > The reason we initially decided to move it into its own file was that C++ 
>> > devirtualization
>> > and function pointer dereferencing/devirtualization will likely not use 
>> > the same analysis.
>> > E.g. ODR only applies to C++, C++ tables are not directly exposed to the 
>> > user.
>> > So we figured that different things should not be merged together, but a 
>> > reuse
>> > of common code to avoid duplication is mandatory.
>>
>> Btw, in other context the idea came up to build candidates based on available
>> API/ABI (that can be indirectly called).  That would help for example the
>> get_ref calls in refine_subpel in the x264 benchmark.  Maybe what you
>> do is actually
>> the very same thing (but look for explicit address-taking) - I didn't
>> look into whether
>> you prune the list of candidates based on API/ABI.
>
>
> No, I don't consider API/ABI at all (do you have a pointer so I can get a 
> better understanding of that idea?).

No, it was just an idea discussed internally.

> Adding guards for all possible functions with the same API/ABI seems 
> expensive (I might misunderstand the idea).
> My patch adds a maximum of 1 test per call site.
>
> What I do is looking which addresses are assigned to the function pointer.
> If there is more than one assigned function, I drop the function pointer from 
> the list of candidates.

OK.  If the program is type correct that's probably going to work well
enough.  If there are more than
one candidates then you could prune those by simple API checks, like
match up the number of arguments
or void vs. non-void return type.  More advanced pruning might lose
some valid candidates (API vs.
ABI compatibility), but it's only heuristic pruning in any case.

It would probably help depending on what exactly "assigned to the
function pointer" means.  If the
function pointer is not from directly visible static storage then
matching up assignments and uses
is going to be a difficult IPA problem itself.  So our original idea was for

 (*fnptr) (args ...);

look for all possible definitions in the (LTO) unit that match the
call signature and that have their
address taken and that possibly could be pointed to by fnptr and if
that's a single one, speculatively
devirtualize that.

> I just checked in the dump file, and the patch also dereferences the indirect 
> calls to get_ref in refine_subpel.

IIRC the x264 case has a global variable with all the function
pointers so your implementation
will likely pick up the single assignment to the individual fields.

Richard.

>
>>
>>
>> > The patch uses the same API like speculative devirtualization in the 
>> > propagation
>> > phase (ipa_make_edge_direct_to_target) and does not do anything in the
>> > transformation phase. So there is no duplication of functionality.
>> >
>> > I will move the code into ipa-devirt.
>> >
>> > Thanks!
>> >
>> >
>> >>
>> >>
>> >> Thanks,
>> >> Richard.
>> >>
>> >> > PR ipa/107666
>> >> > gcc/ChangeLog:
>> >> >
>> >> > * Makefile.in: Add new pass.
>> >> > * common.opt: Add flag -fipa-guarded-deref.
>> >> > * lto-section-in.cc: Add new section "ipa_guarded_deref".
>> >> > * lto-streamer.h (enum lto_section_type): Add new section.
>> >> > * passes.def: Add new pass.
>> >> > * timevar.def (TV_IPA_GUARDED_DEREF): Add time var.
>> >> > * tree-pass.h (make_pass_ipa_guarded_deref): New prototype.
>> >> > * ipa-guarded-deref.cc: New file.
>> >> >
>> >> > Signed-off-by: Christoph Müllner 
>> >> > ---
>> >> >  gcc/Makefile.in  |1 +
>> >> >  gcc/common.opt   |4 +
>> >> >  

Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Christoph Müllner]

On Mon, Nov 14, 2022 at 10:00 AM Richard Biener 
wrote:

> On Mon, Nov 14, 2022 at 9:13 AM Christoph Müllner
>  wrote:
> >
> >
> >
> > On Mon, Nov 14, 2022 at 8:31 AM Richard Biener <
> richard.guent...@gmail.com> wrote:
> >>
> >> On Sun, Nov 13, 2022 at 4:09 PM Christoph Muellner
> >>  wrote:
> >> >
> >> > From: Christoph Müllner 
> >> >
> >> > This patch adds a new pass that looks up function pointer assignments,
> >> > and adds guarded direct calls to the call sites of the function
> >> > pointers.
> >> >
> >> > E.g.: Lets assume an assignment to a function pointer as follows:
> >> > b->cb = 
> >> >   Other part of the program can use the function pointer as
> follows:
> >> > b->cb ();
> >> >   With this pass the invocation will be transformed to:
> >> > if (b->cb == myfun)
> >> >   myfun();
> >> > else
> >> >b->cb ()
> >> >
> >> > The impact of the dynamic guard is expected to be less than the
> speedup
> >> > gained by enabled optimizations (e.g. inlining or constant
> propagation).
> >>
> >> We have speculative devirtualization doing this very transform,
> shouldn't you
> >> instead improve that instead of inventing another specialized pass?
> >
> >
> > Yes, it can be integrated into ipa-devirt.
> >
> > The reason we initially decided to move it into its own file was that
> C++ devirtualization
> > and function pointer dereferencing/devirtualization will likely not use
> the same analysis.
> > E.g. ODR only applies to C++, C++ tables are not directly exposed to the
> user.
> > So we figured that different things should not be merged together, but a
> reuse
> > of common code to avoid duplication is mandatory.
>
> Btw, in other context the idea came up to build candidates based on
> available
> API/ABI (that can be indirectly called).  That would help for example the
> get_ref calls in refine_subpel in the x264 benchmark.  Maybe what you
> do is actually
> the very same thing (but look for explicit address-taking) - I didn't
> look into whether
> you prune the list of candidates based on API/ABI.
>

No, I don't consider API/ABI at all (do you have a pointer so I can get a
better understanding of that idea?).
Adding guards for all possible functions with the same API/ABI seems
expensive (I might misunderstand the idea).
My patch adds a maximum of 1 test per call site.

What I do is looking which addresses are assigned to the function pointer.
If there is more than one assigned function, I drop the function pointer
from the list of candidates.

I just checked in the dump file, and the patch also dereferences the
indirect calls to get_ref in refine_subpel.



>
> > The patch uses the same API like speculative devirtualization in the
> propagation
> > phase (ipa_make_edge_direct_to_target) and does not do anything in the
> > transformation phase. So there is no duplication of functionality.
> >
> > I will move the code into ipa-devirt.
> >
> > Thanks!
> >
> >
> >>
> >>
> >> Thanks,
> >> Richard.
> >>
> >> > PR ipa/107666
> >> > gcc/ChangeLog:
> >> >
> >> > * Makefile.in: Add new pass.
> >> > * common.opt: Add flag -fipa-guarded-deref.
> >> > * lto-section-in.cc: Add new section "ipa_guarded_deref".
> >> > * lto-streamer.h (enum lto_section_type): Add new section.
> >> > * passes.def: Add new pass.
> >> > * timevar.def (TV_IPA_GUARDED_DEREF): Add time var.
> >> > * tree-pass.h (make_pass_ipa_guarded_deref): New prototype.
> >> > * ipa-guarded-deref.cc: New file.
> >> >
> >> > Signed-off-by: Christoph Müllner 
> >> > ---
> >> >  gcc/Makefile.in  |1 +
> >> >  gcc/common.opt   |4 +
> >> >  gcc/ipa-guarded-deref.cc | 1115
> ++
> >> >  gcc/lto-section-in.cc|1 +
> >> >  gcc/lto-streamer.h   |1 +
> >> >  gcc/passes.def   |1 +
> >> >  gcc/timevar.def  |1 +
> >> >  gcc/tree-pass.h  |1 +
> >> >  8 files changed, 1125 insertions(+)
> >> >  create mode 100644 gcc/ipa-guarded-deref.cc
> >> >
> >> > diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> >> > index f672e6ea549..402c4a6ea3f 100644
> >> > --- a/gcc/Makefile.in
> >> > +++ b/gcc/Makefile.in
> >> > @@ -1462,6 +1462,7 @@ OBJS = \
> >> > ipa-sra.o \
> >> > ipa-devirt.o \
> >> > ipa-fnsummary.o \
> >> > +   ipa-guarded-deref.o \
> >> > ipa-polymorphic-call.o \
> >> > ipa-split.o \
> >> > ipa-inline.o \
> >> > diff --git a/gcc/common.opt b/gcc/common.opt
> >> > index bce3e514f65..8344940ae5b 100644
> >> > --- a/gcc/common.opt
> >> > +++ b/gcc/common.opt
> >> > @@ -1933,6 +1933,10 @@ fipa-bit-cp
> >> >  Common Var(flag_ipa_bit_cp) Optimization
> >> >  Perform interprocedural bitwise constant propagation.
> >> >
> >> > +fipa-guarded-deref
> >> > +Common Var(flag_ipa_guarded_deref) Optimization
> >> > +Perform guarded function pointer derferencing.
> >> > +
> >> >  fipa-modref
> >> >  

Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Richard Biener via Gcc-patches]

On Mon, Nov 14, 2022 at 9:13 AM Christoph Müllner
 wrote:
>
>
>
> On Mon, Nov 14, 2022 at 8:31 AM Richard Biener  
> wrote:
>>
>> On Sun, Nov 13, 2022 at 4:09 PM Christoph Muellner
>>  wrote:
>> >
>> > From: Christoph Müllner 
>> >
>> > This patch adds a new pass that looks up function pointer assignments,
>> > and adds guarded direct calls to the call sites of the function
>> > pointers.
>> >
>> > E.g.: Lets assume an assignment to a function pointer as follows:
>> > b->cb = 
>> >   Other part of the program can use the function pointer as follows:
>> > b->cb ();
>> >   With this pass the invocation will be transformed to:
>> > if (b->cb == myfun)
>> >   myfun();
>> > else
>> >b->cb ()
>> >
>> > The impact of the dynamic guard is expected to be less than the speedup
>> > gained by enabled optimizations (e.g. inlining or constant propagation).
>>
>> We have speculative devirtualization doing this very transform, shouldn't you
>> instead improve that instead of inventing another specialized pass?
>
>
> Yes, it can be integrated into ipa-devirt.
>
> The reason we initially decided to move it into its own file was that C++ 
> devirtualization
> and function pointer dereferencing/devirtualization will likely not use the 
> same analysis.
> E.g. ODR only applies to C++, C++ tables are not directly exposed to the user.
> So we figured that different things should not be merged together, but a reuse
> of common code to avoid duplication is mandatory.

Btw, in other context the idea came up to build candidates based on available
API/ABI (that can be indirectly called).  That would help for example the
get_ref calls in refine_subpel in the x264 benchmark.  Maybe what you
do is actually
the very same thing (but look for explicit address-taking) - I didn't
look into whether
you prune the list of candidates based on API/ABI.

> The patch uses the same API like speculative devirtualization in the 
> propagation
> phase (ipa_make_edge_direct_to_target) and does not do anything in the
> transformation phase. So there is no duplication of functionality.
>
> I will move the code into ipa-devirt.
>
> Thanks!
>
>
>>
>>
>> Thanks,
>> Richard.
>>
>> > PR ipa/107666
>> > gcc/ChangeLog:
>> >
>> > * Makefile.in: Add new pass.
>> > * common.opt: Add flag -fipa-guarded-deref.
>> > * lto-section-in.cc: Add new section "ipa_guarded_deref".
>> > * lto-streamer.h (enum lto_section_type): Add new section.
>> > * passes.def: Add new pass.
>> > * timevar.def (TV_IPA_GUARDED_DEREF): Add time var.
>> > * tree-pass.h (make_pass_ipa_guarded_deref): New prototype.
>> > * ipa-guarded-deref.cc: New file.
>> >
>> > Signed-off-by: Christoph Müllner 
>> > ---
>> >  gcc/Makefile.in  |1 +
>> >  gcc/common.opt   |4 +
>> >  gcc/ipa-guarded-deref.cc | 1115 ++
>> >  gcc/lto-section-in.cc|1 +
>> >  gcc/lto-streamer.h   |1 +
>> >  gcc/passes.def   |1 +
>> >  gcc/timevar.def  |1 +
>> >  gcc/tree-pass.h  |1 +
>> >  8 files changed, 1125 insertions(+)
>> >  create mode 100644 gcc/ipa-guarded-deref.cc
>> >
>> > diff --git a/gcc/Makefile.in b/gcc/Makefile.in
>> > index f672e6ea549..402c4a6ea3f 100644
>> > --- a/gcc/Makefile.in
>> > +++ b/gcc/Makefile.in
>> > @@ -1462,6 +1462,7 @@ OBJS = \
>> > ipa-sra.o \
>> > ipa-devirt.o \
>> > ipa-fnsummary.o \
>> > +   ipa-guarded-deref.o \
>> > ipa-polymorphic-call.o \
>> > ipa-split.o \
>> > ipa-inline.o \
>> > diff --git a/gcc/common.opt b/gcc/common.opt
>> > index bce3e514f65..8344940ae5b 100644
>> > --- a/gcc/common.opt
>> > +++ b/gcc/common.opt
>> > @@ -1933,6 +1933,10 @@ fipa-bit-cp
>> >  Common Var(flag_ipa_bit_cp) Optimization
>> >  Perform interprocedural bitwise constant propagation.
>> >
>> > +fipa-guarded-deref
>> > +Common Var(flag_ipa_guarded_deref) Optimization
>> > +Perform guarded function pointer derferencing.
>> > +
>> >  fipa-modref
>> >  Common Var(flag_ipa_modref) Optimization
>> >  Perform interprocedural modref analysis.
>> > diff --git a/gcc/ipa-guarded-deref.cc b/gcc/ipa-guarded-deref.cc
>> > new file mode 100644
>> > index 000..198fb9b33ad
>> > --- /dev/null
>> > +++ b/gcc/ipa-guarded-deref.cc
>> > @@ -0,0 +1,1115 @@
>> > +/* IPA pass to transform indirect calls to guarded direct calls.
>> > +   Copyright (C) 2022 Free Software Foundation, Inc.
>> > +   Contributed by Christoph Muellner (Vrull GmbH)
>> > +   Based on work by Erick Ochoa (Vrull GmbH)
>> > +
>> > +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 

Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Christoph Müllner]

On Mon, Nov 14, 2022 at 8:31 AM Richard Biener 
wrote:

> On Sun, Nov 13, 2022 at 4:09 PM Christoph Muellner
>  wrote:
> >
> > From: Christoph Müllner 
> >
> > This patch adds a new pass that looks up function pointer assignments,
> > and adds guarded direct calls to the call sites of the function
> > pointers.
> >
> > E.g.: Lets assume an assignment to a function pointer as follows:
> > b->cb = 
> >   Other part of the program can use the function pointer as follows:
> > b->cb ();
> >   With this pass the invocation will be transformed to:
> > if (b->cb == myfun)
> >   myfun();
> > else
> >b->cb ()
> >
> > The impact of the dynamic guard is expected to be less than the speedup
> > gained by enabled optimizations (e.g. inlining or constant propagation).
>
> We have speculative devirtualization doing this very transform, shouldn't
> you
> instead improve that instead of inventing another specialized pass?
>

Yes, it can be integrated into ipa-devirt.

The reason we initially decided to move it into its own file was that C++
devirtualization
and function pointer dereferencing/devirtualization will likely not use the
same analysis.
E.g. ODR only applies to C++, C++ tables are not directly exposed to the
user.
So we figured that different things should not be merged together, but a
reuse
of common code to avoid duplication is mandatory.

The patch uses the same API like speculative devirtualization in the
propagation
phase (ipa_make_edge_direct_to_target) and does not do anything in the
transformation phase. So there is no duplication of functionality.

I will move the code into ipa-devirt.

Thanks!



>
> Thanks,
> Richard.
>
> > PR ipa/107666
> > gcc/ChangeLog:
> >
> > * Makefile.in: Add new pass.
> > * common.opt: Add flag -fipa-guarded-deref.
> > * lto-section-in.cc: Add new section "ipa_guarded_deref".
> > * lto-streamer.h (enum lto_section_type): Add new section.
> > * passes.def: Add new pass.
> > * timevar.def (TV_IPA_GUARDED_DEREF): Add time var.
> > * tree-pass.h (make_pass_ipa_guarded_deref): New prototype.
> > * ipa-guarded-deref.cc: New file.
> >
> > Signed-off-by: Christoph Müllner 
> > ---
> >  gcc/Makefile.in  |1 +
> >  gcc/common.opt   |4 +
> >  gcc/ipa-guarded-deref.cc | 1115 ++
> >  gcc/lto-section-in.cc|1 +
> >  gcc/lto-streamer.h   |1 +
> >  gcc/passes.def   |1 +
> >  gcc/timevar.def  |1 +
> >  gcc/tree-pass.h  |1 +
> >  8 files changed, 1125 insertions(+)
> >  create mode 100644 gcc/ipa-guarded-deref.cc
> >
> > diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> > index f672e6ea549..402c4a6ea3f 100644
> > --- a/gcc/Makefile.in
> > +++ b/gcc/Makefile.in
> > @@ -1462,6 +1462,7 @@ OBJS = \
> > ipa-sra.o \
> > ipa-devirt.o \
> > ipa-fnsummary.o \
> > +   ipa-guarded-deref.o \
> > ipa-polymorphic-call.o \
> > ipa-split.o \
> > ipa-inline.o \
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index bce3e514f65..8344940ae5b 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -1933,6 +1933,10 @@ fipa-bit-cp
> >  Common Var(flag_ipa_bit_cp) Optimization
> >  Perform interprocedural bitwise constant propagation.
> >
> > +fipa-guarded-deref
> > +Common Var(flag_ipa_guarded_deref) Optimization
> > +Perform guarded function pointer derferencing.
> > +
> >  fipa-modref
> >  Common Var(flag_ipa_modref) Optimization
> >  Perform interprocedural modref analysis.
> > diff --git a/gcc/ipa-guarded-deref.cc b/gcc/ipa-guarded-deref.cc
> > new file mode 100644
> > index 000..198fb9b33ad
> > --- /dev/null
> > +++ b/gcc/ipa-guarded-deref.cc
> > @@ -0,0 +1,1115 @@
> > +/* IPA pass to transform indirect calls to guarded direct calls.
> > +   Copyright (C) 2022 Free Software Foundation, Inc.
> > +   Contributed by Christoph Muellner (Vrull GmbH)
> > +   Based on work by Erick Ochoa (Vrull GmbH)
> > +
> > +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
> > +.  */
> > +
> > +/* Indirect calls are used to separate callees from their call sites.
> > +   This helps to implement proper abstraction layers, but prevents
> > +   optimizations like constant-propagation or function specialization.
> > 

Re: [RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Richard Biener via Gcc-patches]

On Sun, Nov 13, 2022 at 4:09 PM Christoph Muellner
 wrote:
>
> From: Christoph Müllner 
>
> This patch adds a new pass that looks up function pointer assignments,
> and adds guarded direct calls to the call sites of the function
> pointers.
>
> E.g.: Lets assume an assignment to a function pointer as follows:
> b->cb = 
>   Other part of the program can use the function pointer as follows:
> b->cb ();
>   With this pass the invocation will be transformed to:
> if (b->cb == myfun)
>   myfun();
> else
>b->cb ()
>
> The impact of the dynamic guard is expected to be less than the speedup
> gained by enabled optimizations (e.g. inlining or constant propagation).

We have speculative devirtualization doing this very transform, shouldn't you
instead improve that instead of inventing another specialized pass?

Thanks,
Richard.

> PR ipa/107666
> gcc/ChangeLog:
>
> * Makefile.in: Add new pass.
> * common.opt: Add flag -fipa-guarded-deref.
> * lto-section-in.cc: Add new section "ipa_guarded_deref".
> * lto-streamer.h (enum lto_section_type): Add new section.
> * passes.def: Add new pass.
> * timevar.def (TV_IPA_GUARDED_DEREF): Add time var.
> * tree-pass.h (make_pass_ipa_guarded_deref): New prototype.
> * ipa-guarded-deref.cc: New file.
>
> Signed-off-by: Christoph Müllner 
> ---
>  gcc/Makefile.in  |1 +
>  gcc/common.opt   |4 +
>  gcc/ipa-guarded-deref.cc | 1115 ++
>  gcc/lto-section-in.cc|1 +
>  gcc/lto-streamer.h   |1 +
>  gcc/passes.def   |1 +
>  gcc/timevar.def  |1 +
>  gcc/tree-pass.h  |1 +
>  8 files changed, 1125 insertions(+)
>  create mode 100644 gcc/ipa-guarded-deref.cc
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index f672e6ea549..402c4a6ea3f 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1462,6 +1462,7 @@ OBJS = \
> ipa-sra.o \
> ipa-devirt.o \
> ipa-fnsummary.o \
> +   ipa-guarded-deref.o \
> ipa-polymorphic-call.o \
> ipa-split.o \
> ipa-inline.o \
> diff --git a/gcc/common.opt b/gcc/common.opt
> index bce3e514f65..8344940ae5b 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1933,6 +1933,10 @@ fipa-bit-cp
>  Common Var(flag_ipa_bit_cp) Optimization
>  Perform interprocedural bitwise constant propagation.
>
> +fipa-guarded-deref
> +Common Var(flag_ipa_guarded_deref) Optimization
> +Perform guarded function pointer derferencing.
> +
>  fipa-modref
>  Common Var(flag_ipa_modref) Optimization
>  Perform interprocedural modref analysis.
> diff --git a/gcc/ipa-guarded-deref.cc b/gcc/ipa-guarded-deref.cc
> new file mode 100644
> index 000..198fb9b33ad
> --- /dev/null
> +++ b/gcc/ipa-guarded-deref.cc
> @@ -0,0 +1,1115 @@
> +/* IPA pass to transform indirect calls to guarded direct calls.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   Contributed by Christoph Muellner (Vrull GmbH)
> +   Based on work by Erick Ochoa (Vrull GmbH)
> +
> +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
> +.  */
> +
> +/* Indirect calls are used to separate callees from their call sites.
> +   This helps to implement proper abstraction layers, but prevents
> +   optimizations like constant-propagation or function specialization.
> +
> +   Assuming that we identify a function pointer that gets assigned
> +   only a small amount of times, we can convert the indirect calls
> +   to the target function into guarded direct calls and let later
> +   passes apply additional optimizations.
> +
> +   This pass does this by:
> +   * Identifying function pointers that are assigned up to N=1 times
> + to struct fields.
> +   * Convert the indirect calls into a test for the call target
> + and a direct call
> +   * If the test fails, then the indirect call will be executed.
> +
> +   E.g.:
> +   - function foo's address is taken and stored in a field of struct
> +   o->func = foo;
> +   - the program writes into this struct field only once
> +   - it is possible, that we miss a store (we would need strong guarantees)
> + therefore, we do the following conversion:
> +   o->func ()
> + <-->
> +   if (o->func == foo)
> +foo ()
> +   else
> +o->func ()
> +
> +   This 

[RFC PATCH] ipa-guarded-deref: Add new pass to dereference function pointers

[Christoph Muellner]

From: Christoph Müllner 

This patch adds a new pass that looks up function pointer assignments,
and adds guarded direct calls to the call sites of the function
pointers.

E.g.: Lets assume an assignment to a function pointer as follows:
b->cb = 
  Other part of the program can use the function pointer as follows:
b->cb ();
  With this pass the invocation will be transformed to:
if (b->cb == myfun)
  myfun();
else
   b->cb ()

The impact of the dynamic guard is expected to be less than the speedup
gained by enabled optimizations (e.g. inlining or constant propagation).

PR ipa/107666
gcc/ChangeLog:

* Makefile.in: Add new pass.
* common.opt: Add flag -fipa-guarded-deref.
* lto-section-in.cc: Add new section "ipa_guarded_deref".
* lto-streamer.h (enum lto_section_type): Add new section.
* passes.def: Add new pass.
* timevar.def (TV_IPA_GUARDED_DEREF): Add time var.
* tree-pass.h (make_pass_ipa_guarded_deref): New prototype.
* ipa-guarded-deref.cc: New file.

Signed-off-by: Christoph Müllner 
---
 gcc/Makefile.in  |1 +
 gcc/common.opt   |4 +
 gcc/ipa-guarded-deref.cc | 1115 ++
 gcc/lto-section-in.cc|1 +
 gcc/lto-streamer.h   |1 +
 gcc/passes.def   |1 +
 gcc/timevar.def  |1 +
 gcc/tree-pass.h  |1 +
 8 files changed, 1125 insertions(+)
 create mode 100644 gcc/ipa-guarded-deref.cc

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index f672e6ea549..402c4a6ea3f 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1462,6 +1462,7 @@ OBJS = \
ipa-sra.o \
ipa-devirt.o \
ipa-fnsummary.o \
+   ipa-guarded-deref.o \
ipa-polymorphic-call.o \
ipa-split.o \
ipa-inline.o \
diff --git a/gcc/common.opt b/gcc/common.opt
index bce3e514f65..8344940ae5b 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1933,6 +1933,10 @@ fipa-bit-cp
 Common Var(flag_ipa_bit_cp) Optimization
 Perform interprocedural bitwise constant propagation.
 
+fipa-guarded-deref
+Common Var(flag_ipa_guarded_deref) Optimization
+Perform guarded function pointer derferencing.
+
 fipa-modref
 Common Var(flag_ipa_modref) Optimization
 Perform interprocedural modref analysis.
diff --git a/gcc/ipa-guarded-deref.cc b/gcc/ipa-guarded-deref.cc
new file mode 100644
index 000..198fb9b33ad
--- /dev/null
+++ b/gcc/ipa-guarded-deref.cc
@@ -0,0 +1,1115 @@
+/* IPA pass to transform indirect calls to guarded direct calls.
+   Copyright (C) 2022 Free Software Foundation, Inc.
+   Contributed by Christoph Muellner (Vrull GmbH)
+   Based on work by Erick Ochoa (Vrull GmbH)
+
+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
+.  */
+
+/* Indirect calls are used to separate callees from their call sites.
+   This helps to implement proper abstraction layers, but prevents
+   optimizations like constant-propagation or function specialization.
+
+   Assuming that we identify a function pointer that gets assigned
+   only a small amount of times, we can convert the indirect calls
+   to the target function into guarded direct calls and let later
+   passes apply additional optimizations.
+
+   This pass does this by:
+   * Identifying function pointers that are assigned up to N=1 times
+ to struct fields.
+   * Convert the indirect calls into a test for the call target
+ and a direct call
+   * If the test fails, then the indirect call will be executed.
+
+   E.g.:
+   - function foo's address is taken and stored in a field of struct
+   o->func = foo;
+   - the program writes into this struct field only once
+   - it is possible, that we miss a store (we would need strong guarantees)
+ therefore, we do the following conversion:
+   o->func ()
+ <-->
+   if (o->func == foo)
+foo ()
+   else
+o->func ()
+
+   This pass is implemented as a full IPA pass that uses the LTO section
+   "ipa_guarded_deref".  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "gimple.h"
+#include "alloc-pool.h"
+#include "tree-pass.h"
+#include "tree-cfg.h"
+#include "ssa.h"
+#include "cgraph.h"
+#include "gimple-pretty-print.h"
+#include "gimple-iterator.h"
+#include "symbol-summary.h"
+#include "ipa-utils.h"
+
+#include