| From: | Pierre Ducroquet <p(dot)psql(at)pinaraf(dot)info> |
|---|---|
| To: | Matheus Alcantara <matheusssilv97(at)gmail(dot)com> |
| Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [PATCH] llvmjit: always add the simplifycfg pass |
| Date: | 2026-01-28 07:56:46 |
| Message-ID: | TyPuJ3RPE7iMOji1DSq1IIHlq_RtGBgG5YrJJyeocYIxWMnlx8EsW93R_qdb9uYYofmelyF4nrP1rao5RrWKYSUyi3BlSO9ZpzgHzUvsJH4=@pinaraf.info |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi
Here is a rebased version of the patch with a rewrite of the comment.
Thank you again for your previous review.
FYI, I've tried adding other passes but none had a similar benefits over cost ratio. The benefits could rather be in changing from O3 to an extensive list of passes.
Le jeudi 22 janvier 2026 à 10:41 PM, Matheus Alcantara <matheusssilv97(at)gmail(dot)com> a écrit :
> On Thu Jan 22, 2026 at 5:27 PM -03, Pierre Ducroquet wrote:
>
> > > The patch needs a rebase due to e5d99b4d9ef.
> > >
> > > You've added the "simplifycfg" only when the "jit_optimize_above_cost"
> > > is not triggered which will use the default<O0> and mem2reg passes, the
> > >
> > > default<O3> pass already include "simplifycfg"?
> > >
> > > With e5d99b4d9ef being committed, should we add "simplifycfg" when
> > > PGJIT_INLINE bit is set since it also use the default<O0> and mem2reg
> > >
> > > passes?
> >
> > Hi
> >
> > Thank you, here is a rebased version of the patch.
> > To answer your questions:
> > - O3 already includes simplifycfg, so no need to modify O3
> > - any code generated by our llvmjit provider, esp. tuple deforming, is heavily dependent on simplifycfg, so when O0 is the basis we should always add this pass
>
>
> Thanks for confirming.
>
> I did some benchmarks on some TPCH queries (1 and 4) and I got these
> results. Note that for these tests I set jit_optimize_above_cost=1000000
> so that it force to use the default<O0> pass with simplifycfg.
>
>
> Master Q1:
> Timing: Generation 1.553 ms (Deform 0.573 ms), Inlining 0.052 ms, Optimization 95.571 ms, Emission 58.941 ms, Total 156.116 ms
> Execution Time: 38221.318 ms
>
> Patch Q1:
> Timing: Generation 1.477 ms (Deform 0.534 ms), Inlining 0.040 ms, Optimization 95.364 ms, Emission 58.046 ms, Total 154.927 ms
> Execution Time: 38257.797 ms
>
> Master Q4:
> Timing: Generation 0.836 ms (Deform 0.309 ms), Inlining 0.086 ms, Optimization 5.098 ms, Emission 6.963 ms, Total 12.983 ms
> Execution Time: 19512.134 ms
>
> Patch Q4:
> Timing: Generation 0.802 ms (Deform 0.294 ms), Inlining 0.090 ms, Optimization 5.234 ms, Emission 6.521 ms, Total 12.648 ms
> Execution Time: 16051.483 ms
>
>
> For Q4 I see a small increase on Optimization phase but we have a good
> performance improvement on execution time. For Q1 the results are almost
> the same.
>
> I did not find any major regression using simplifycfg pass and I think
> that it make sense to enable since it generate better IR code for LLVM
> to compile without too much costs. +1 for this patch.
>
> Perhaps we could merge the comments on if/else block to include the
> simplifycfg, what do you think?
>
> + /*
> + * Determine the LLVM pass pipeline to use. For OPT3 we use the standard
> + * suite. For lower optimization levels, we explicitly include mem2reg to
> + * promote stack variables, simplifycfg to clean up the control flow , and
> + * optionally the inliner if the flag is set. Note that default<O0> already
>
> + * includes the always-inline pass.
> + */
> if (context->base.flags & PGJIT_OPT3)
>
> passes = "default<O3>";
>
> else if (context->base.flags & PGJIT_INLINE)
>
> - /* if doing inlining, but no expensive optimization, add inline pass */
> passes = "default<O0>,mem2reg,simplifycfg,inline";
>
> else
> - /* default<O0> includes always-inline pass */
>
> passes = "default<O0>,mem2reg,simplifycfg";
>
>
> --
> Matheus Alcantara
> EDB: https://www.enterprisedb.com
>
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-llvmjit-always-use-the-simplifycfg-pass.patch | text/x-patch | 3.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2026-01-28 08:06:59 | Re: Extended Statistics set/restore/clear functions. |
| Previous Message | Chao Li | 2026-01-28 07:35:22 | Re: display hot standby state in psql prompt |