[llvm] [GlobalISel] Optimized MatchData Handling (PR #92115)

Pierre van Houtryve via llvm-commits llvm-commits at lists.llvm.org
Tue May 14 23:55:51 PDT 2024


Pierre-vh wrote:

> > > One related thing I have never understood is, why is every combine split into a "match" function and an "apply" function? Aren't they always effectively called in tandem like this:
> > > ```
> > >   if (matchFunc(MatchData))
> > >     applyFunc(MatchData);
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > ? Is there a use case for calling "match" without "apply"?
> > 
> > 
> > Matchers can fail/reject after doing further analysis (e.g. calling KnowBits), so the matcher is like an analysis part and the apply is the actual transform.
> 
> Right, but why not always compile the match+apply as a single C++ function like:
> 
> ```
>   bool matchAndApply(MI) {
>     // match part goes here
> 
>     if (analysis failed)
>       return false;
> 
>     // apply part goes here
>     return true;
>   }
> ```
> 
> Then you don't have to worry about managing the lifetime of matchdata objects outside of this function.

That's interesting, I'm not sure if it's possible to do that but I'll add that to my list.



> I'll get some perf numbers on this change, thanks.
> 
> Also curious: do you think there's any significant problem with our common use of the build function lambdas?

I haven't seen issues related to the build lambdas, no

As for the (size and runtime) regression, that's really weird. `perf record` led me to believe this would improve things.
The size regression is probably due to inlining of the `getOrCreateMatchData` all over the place indeed.

I'm wondering if the pointer cast + indirection to reach the MatchData is adding too much overhead, I'll rethink this and try to understand why

https://github.com/llvm/llvm-project/pull/92115


More information about the llvm-commits mailing list