Re: [PATCH] llvmjit: always add the simplifycfg pass - Mailing list pgsql-hackers

On 30/01/26 12:01, Pierre Ducroquet wrote:
> Le jeudi 29 janvier 2026 à 12:19 AM, Andres Freund <andres@anarazel.de> a écrit :
> 
>> Hi,
>>
>> On 2026-01-28 07:56:46 +0000, Pierre Ducroquet wrote:
>>
>>> 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.
>>
>>
>> I agree that we should have a better list of passes. I'm a bit worried that
>> having an explicit list of passes that we manage ourselves is going to be
>> somewhat of a pain to maintain across llvm versions, but ...
>>
>> WRT passes that might be worth having even with -O0 - running duplicate
>> function merging early on could be quite useful, particularly because we won't
>> inline the deform routines anyway.
>>
>>>> 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.
>>
>> ...
>>
>> These queries are all simple enough that I'm not sure this is a particularly
>> good benchmark for optimization speed. In particular, the deform routines
>> don't have to deal with a lot of columns and there aren't a lot of functions
>> (although I guess that shouldn't really matter WRT simplifycfg).
>>
> 
> simplifycfg seems to do more things on the deforming functions than I anticipated initially, explaining the
performancebenefits. I've written patches to our C code to generate better IR, but I discovered quite a puzzle.
 
> The biggest gain I see on the generated amd64 code for a very simple query (SELECT * FROM demo WHERE a = 42) with
simplifycfgis that it prevents spilling on the stack and it does what mem2reg was supposed to be doing.
 
> 
> 
> Running opt -debug-pass-manager on a deform function, I get:
> - with default<O0>,mem2reg
> 
> Running pass: AnnotationRemarksPass on deform_0_1 (56 instructions)
> Running analysis: TargetLibraryAnalysis on deform_0_1
> Running pass: PromotePass on deform_0_1 (56 instructions)
> Running analysis: DominatorTreeAnalysis on deform_0_1
> Running analysis: AssumptionAnalysis on deform_0_1
> Running analysis: TargetIRAnalysis on deform_0_1
> 
> deform_0_1:                             # @deform_0_1
>          .cfi_startproc
> # %bb.0:                                # %entry
>          movq    24(%rdi), %rax
>          movq    %rax, -48(%rsp)                 # 8-byte Spill
>          movq    32(%rdi), %rax
>          movq    %rax, -40(%rsp)                 # 8-byte Spill
>          movq    %rdi, %rax
>          addq    $4, %rax
>          movq    %rax, -32(%rsp)                 # 8-byte Spill
>          movq    %rdi, %rax
>          addq    $6, %rax
>          movq    %rax, -24(%rsp)                 # 8-byte Spill
>          movq    %rdi, %rax
>          addq    $72, %rax
>          movq    %rax, -16(%rsp)                 # 8-byte Spill
> ...
> 
> 
> 
> - with default<O0>,simplifycfg
> 
> Running pass: AnnotationRemarksPass on deform_0_1 (56 instructions)
> Running analysis: TargetLibraryAnalysis on deform_0_1
> Running pass: SimplifyCFGPass on deform_0_1 (56 instructions)
> Running analysis: TargetIRAnalysis on deform_0_1
> Running analysis: AssumptionAnalysis on deform_0_1
> 
> deform_0_1:                             # @deform_0_1
>          .cfi_startproc
> # %bb.0:                                # %entry
>          movq    24(%rdi), %rax
>          movq    32(%rdi), %rsi
>          movq    64(%rdi), %rcx
>          movq    16(%rcx), %rcx
>          movzbl  22(%rcx), %edx
>          movslq  %edx, %rdx
>          addq    %rdx, %rcx
>          movl    72(%rdi), %edx
> ...
> 
> - with default<O0>,simplifycfg,mem2reg
> 
> Running pass: SimplifyCFGPass on deform_0_1 (56 instructions)
> Running analysis: TargetIRAnalysis on deform_0_1
> Running analysis: AssumptionAnalysis on deform_0_1
> Running pass: PromotePass on deform_0_1 (46 instructions)
> Running analysis: DominatorTreeAnalysis on deform_0_1
> 
> deform_0_1:                             # @deform_0_1
>          .cfi_startproc
> # %bb.0:                                # %entry
>          movq    24(%rdi), %rax
>          movq    32(%rdi), %rsi
>          movq    64(%rdi), %rcx
>          movq    16(%rcx), %rcx
>          movzbl  22(%rcx), %edx
>          movb    $0, (%rsi)
> ...
> 
> 
> So even when running only simplifycfg, the stack allocation goes away.
> I am trying to figure that one out, but I suspect we are no longer doing the optimizations we thought we were doing
withmem2reg only, hence the (surprising) speed gains with simplifycfg.
 
> 

I did some tests to compare the IR output with different pass
combinations. Using a query that deforms 6 columns, the raw IR generates
trivial empty blocks like this:

block.attr.0.attcheckalign:           ; preds = %block.attr.0.start
  br label %block.attr.0.align

block.attr.0.align:                   ; preds = %block.attr.0.attcheckalign
  br label %block.attr.0.store

block.attr.0.store:                   ; preds = %block.attr.0.align
  %26 = load i64, ptr %v_offp, align 8
  %27 = getelementptr i8, ptr %v_tupdata_base, i64 %26
  ...

With mem2reg only, the alloca is promoted but these empty blocks remain:

block.attr.0.attcheckalign:           ; preds = %block.attr.0.start
  br label %block.attr.0.align

block.attr.0.align:                   ; preds = %block.attr.0.attcheckalign
  br label %block.attr.0.store

block.attr.0.store:                   ; preds = %block.attr.0.align
  %25 = getelementptr i8, ptr %v_tupdata_base, i64 0
  ...

With simplifycfg only, trivial blocks are merged but alloca is not
promoted:

block.attr.0.start:                               ; preds = %block.attr.0.attcheckattno
  %21 = getelementptr i8, ptr %8, i32 0
  %attnullbyte = load i8, ptr %21, align 1
  %22 = and i8 %attnullbyte, 1
  %attisnull = icmp eq i8 %22, 0
  %23 = and i1 %hasnulls, %attisnull
  br i1 %23, label %block.attr.0.attisnull, label %block.attr.0.store

block.attr.0.store:                               ; preds = %block.attr.0.start
  %26 = load i64, ptr %v_offp, align 8
  %27 = getelementptr i8, ptr %v_tupdata_base, i64 %26
  ...

After mem2reg,simplifycfg the trivial blocks are merged and block.attr.0.start
branches directly to block.attr.0.store:

block.attr.0.start:                   ; preds = %block.attr.0.attcheckattno
  %20 = getelementptr i8, ptr %8, i32 0
  %attnullbyte = load i8, ptr %20, align 1
  %21 = and i8 %attnullbyte, 1
  %attisnull = icmp eq i8 %21, 0
  %22 = and i1 %hasnulls, %attisnull
  br i1 %22, label %block.attr.0.attisnull, label %block.attr.0.store

block.attr.0.store:                   ; preds = %block.attr.0.start
  %25 = getelementptr i8, ptr %v_tupdata_base, i64 0
  ...

As the simplifycfg[1] may remove basic blocks and eliminate PHI nodes,
perhaps this enables more registers to be used and avoid stack
allocations? It seems to me that the stack allocation going away on your
example may be a side-effect of the simpler CFG allowing better register
allocation. However, I think that mem2reg is still needed since
simplifycfg alone doesn't promote allocas, the two passes complement
each other.

What do you think?

[1] https://llvm.org/docs/Passes.html#simplifycfg-simplify-the-cfg

--
Matheus Alcantara
EDB: https://www.enterprisedb.com