GH-50371: [C++][Gandiva] Fold common subexpressions before code generation

[likun666661]

Rationale for this change

This adds a conservative Gandiva expression-layer common subexpression folding pass before code generation. Repeated pure expression subtrees can otherwise be decomposed and lowered multiple times before LLVM sees the module.

The pass intentionally avoids cases where expression-level reuse can be unsafe, including functions that need execution context/function holders, can return errors, use result-null-internal handling, and boolean/if decomposition results with local validity bitmap side effects.

What changes are included in this PR?

• Add expr_cse to fold safe repeated Gandiva expression subtrees before projector/filter code generation.
• Add decomposition reuse for safe field, literal, and pure function nodes.
• Expose unoptimized IR dumping for Gandiva tests when dump_ir is enabled.
• Add IR-focused tests covering nested arithmetic, generated if, generated boolean, and nested between-style patterns.

Are these changes tested?

Yes.

$ git diff --check
$ pre-commit run --show-diff-on-failure --color=always --all-files cpp
$ PYTHON=/opt/homebrew/bin/python3 TZ=UTC ARROW_TEST_DATA=/Users/likun/workspace-for-apache-arrow/testing/data ninja -C cpp/build-gandiva unittest

The ninja ... unittest run completed with 100% tests passed, 0 tests failed out of 81.

AI-assisted contribution disclosure

This PR was prepared with AI assistance. I reviewed and tested the generated changes locally, including the Gandiva C++ tests and Arrow C++ pre-commit checks listed above.

• GitHub Issue: [C++][Gandiva] Add conservative common subexpression elimination before code generation #50371

[github-actions]

⚠️ GitHub issue #50371 has no components, please add labels for components.

[kou]

@dmitry-chirkov-dremio @lriggs @akravchukdremio @xxlaykxx Could you review this?

[Copilot]

Pull request overview

This PR introduces a conservative expression-layer common subexpression folding (CSE) pass in Gandiva before LLVM code generation, aiming to reduce redundant decomposition/codegen for repeated safe/pure subtrees while explicitly avoiding reuse in cases where Gandiva’s validity/bitmap side effects could affect correctness. It also adds the ability to dump unoptimized IR (pre-optimizer) to support targeted IR-based tests.

Changes:

• Add expr_cse folding pass and apply it in both Projector::Make and Filter::Make before validation, cache-keying, and code generation.
• Add decomposition-time reuse for safe nodes (fields, literals, and pure/native functions that don’t need context/holders and can’t error / use null-internal).
• Add unoptimized IR dumping plumbing and new IR-focused regression tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
cpp/src/gandiva/tests/projector_test.cc	Adds IR-string-based tests validating CSE behavior (optimized vs unoptimized IR) for repeated arithmetic, generated if, generated boolean, and nested between-like patterns.
cpp/src/gandiva/projector.h	Exposes DumpUnoptimizedIR() on Projector.
cpp/src/gandiva/projector.cc	Folds common subexpressions before building cache keys, validating, and building the LLVM module; wires DumpUnoptimizedIR().
cpp/src/gandiva/llvm_generator.h	Exposes unoptimized_ir() passthrough.
cpp/src/gandiva/filter.cc	Folds common subexpressions before cache-keying, validation, and building for filters.
cpp/src/gandiva/expr_decomposer.h / .cc	Adds decomposition caching for reuse-safe nodes to avoid repeated decomposition work.
cpp/src/gandiva/expr_cse.h / .cc	Introduces the conservative expression-tree folding pass and structural interning for safe nodes.
cpp/src/gandiva/engine.h / .cc	Captures and exposes module IR before the optimizer pipeline when dump_ir is enabled.
cpp/src/gandiva/CMakeLists.txt	Adds expr_cse.cc to the build.

[dmitry-chirkov-dremio]

Asked Codex to walk me through testing of the changes:

The important missing tests are precisely the “do not fold” rules:

• kResultNullInternal
• NeedsContext()
• NeedsFunctionHolder()
• CanReturnErrors()
• A safe parent containing one unsafe child
• Unknown/custom registry functions
• Opaque InExpressionNode
• Structurally similar but unequal expressions
• Different literal values/types
• Operand order, such as add(a,b) versus add(b,a)
• Multiple output expressions sharing a subtree
• Custom volatile or nondeterministic functions with default flags

There are no focused unit tests for FoldCommonSubexpressions() itself. Everything is tested indirectly through Projector/Filter and IR strings. The positive rewrite coverage is respectable, but the conservative safety contract is largely untested.

[dmitry-chirkov-dremio]

Also asked Codex to walk me through large expression and performance coverage:

There is no new large expression test or benchmark in the PR.

The repository already has:

• TimedTestBigNested — approximately 20 nested if expressions, but Projector construction occurs outside the timed loop, so it measures evaluation rather than CSE/build cost.
• TimedTestExprCompilation — measures Projector construction, but its expressions do not contain meaningfully large repeated subtrees.

Neither benchmark was changed, and the PR does not report running either one.

Seems like a meaningful gap.
The PR provides no evidence that:

• Projector/Filter construction becomes faster for repeated expressions.
• Generated IR becomes smaller.
• Runtime evaluation improves.
• The pass does not regress ordinary expressions with no duplicates.

I'd love to see before/after measurements for at least:

1. A deep expression with no duplicates - worst-case overhead with no benefit
2. A balanced expression containing many repeated safe subtrees
3. Repeated unsafe subtrees that must not fold
4. Several sizes, such as 10, 100, and 1,000 nodes
5. Projector/Filter build time and generated IR size; evaluation time separately

ps I'll ask our team if we have real world examples of "large expressions" - we typically see them in/around CASE expressions where individual branches are semi-repeated expanded nested calcs

[likun666661]

Thank you for the detailed and constructive feedback. I agree with the concerns you raised, especially that a conservative CSE pass needs both a clearly tested safety contract and evidence that it improves the workloads it targets without regressing ordinary expressions.

I'm happy to continue working on this. My initial direction was inspired by this article about Apache Cloudberry's Gandiva optimization work: Apache Cloudberry vectorized execution practice (3): Gandiva optimization. It discusses repeated expression construction, structural deduplication with expression DAGs, deterministic semantic hashes, and reports meaningful reductions in expression nodes and construction time.

That said, the article describes an optimization at the expression-construction layer, while this PR currently applies folding inside Gandiva before validation and code generation. I agree that we should validate this implementation independently rather than assume that the same results carry over.

My proposed next steps are:

1. Make unoptimized-IR availability depend on the IR actually captured by Engine, including the mutable-Configuration regression case.
2. Add focused unit tests for FoldCommonSubexpressions, especially every "do not fold" condition and the structurally unequal cases you listed.
3. Revisit the purity/determinism contract for custom functions. The default-flags volatile-function case is a real gap; absence of the existing flags should not by itself be treated as proof that a function is safe to reuse.
4. Add large-expression benchmarks for no-duplicate, repeated-safe, and repeated-unsafe trees at several sizes, measuring Projector/Filter construction and IR size separately from evaluation.
5. Share the before/after numbers and adjust or narrow the implementation based on the results.

If your team can share representative CASE-shaped expressions from real workloads, I would also be glad to incorporate them into the benchmark cases.

Thanks again for taking the time to walk through this so carefully. The feedback is very helpful, and I'd like to keep pushing the work forward.

[likun666661]

Thank you again for the detailed review. I have completed a local follow-up
prototype and benchmark pass addressing the two remaining concerns: the conservative
"do not fold" contract and large-expression performance evidence.

I have not pushed these changes yet. I would like to confirm the narrowed design
with you before updating the PR.

Proposed narrowed design

I split the implementation into two conservative layers:

1. AST structural sharing

   • Canonicalize structurally identical fields, literals, and safe built-in function
     subtrees into a DAG.
   • Use a structural key composed of node kind, function/field/literal identity,
     return type, and ordered child atom IDs.
   • Preserve operand order. There is no commutative normalization.

2. Same-basic-block Dex/LLVM value reuse

   • Reuse a decomposed ValueValidityPair only for a reusable canonical AST node.
   • Reuse a generated LValue only when the same Dex was generated in the same
     LLVM BasicBlock.
   • Cache a generated value only when code generation ends in the block where it
     started.

I removed the Boolean/if algebraic transformations from the local version:

• no Boolean flattening or predicate deduplication;
• no if(c, x, x) -> x;
• no reuse across Boolean short-circuit blocks or if branches;
• no reuse across separately compiled output expressions.

This is intentionally a post-construction Gandiva pass. It canonicalizes an AST after
the caller has already built it in Projector::Make() or Filter::Make(). It reduces
decomposition, generated IR, LLVM optimization, and JIT work, but does not claim to
remove caller-side TreeExprBuilder or SQL-to-Gandiva construction cost. That is an
important difference from the Cloudberry article that initially motivated this work.

Safety contract

A function subtree is reusable only when:

the function is a Gandiva built-in
and result nullability is not kResultNullInternal
and it does not need ExecutionContext
and it does not need a FunctionHolder
and it cannot return errors
and every child subtree is reusable

The built-in-only rule prevents a custom function registered with default flags from
being treated as pure accidentally.

Functions such as lower and upper remain excluded in this initial version. They are
deterministic at the SQL level, but Gandiva marks them kNeedsContext; their
implementations allocate output from the execution-context arena and may set errors for
invalid UTF-8, invalid lengths, or allocation failures. I think those functions need
explicit effect/CSE-safety metadata rather than relaxing the context restriction
globally.

Affected modules

Module	Responsibility
expr_cse.{h,cc}	Structural keys, canonical AST nodes, shared safety predicate
function_registry.{h,cc}	Distinguish built-ins from custom registered functions
projector.cc, filter.cc	Fold before cache lookup, validation, and code generation
expr_decomposer.{h,cc}	Cache safe decompositions by canonical node identity
llvm_generator.{h,cc}	Reuse generated LValue only in the same basic block
engine.h, projector.cc	Guard unoptimized IR using actual captured engine state
CSE/projector/filter tests	Safety-contract, IR-shape, runtime, and cache regressions
micro_benchmarks.cc	Fold, build, IR-size, and evaluation benchmarks

Module flow

flowchart LR
  Caller["Gandiva caller"]
  Entry["Projector::Make / Filter::Make"]
  Folder["AST structural CSE"]
  Registry["FunctionRegistry safety/provenance"]
  Cache["Gandiva object cache"]
  Decomposer["ExprDecomposer"]
  Generator["LLVMGenerator::Visitor"]
  BlockCache["Basic-block Dex cache"]
  Engine["LLVM optimizer and JIT"]

  Caller --> Entry
  Entry --> Folder
  Folder --> Registry
  Entry --> Cache
  Entry --> Decomposer
  Decomposer --> Registry
  Decomposer --> Generator
  Generator --> BlockCache
  Generator --> Engine

Loading

Build sequence

sequenceDiagram
  participant C as Caller
  participant P as Projector or Filter
  participant F as AST folder
  participant R as FunctionRegistry
  participant D as ExprDecomposer
  participant G as LLVMGenerator
  participant E as Engine

  C->>P: Make(schema, expressions, config)
  P->>F: FoldCommonSubexpressions
  loop bottom-up AST traversal
    F->>R: LookupSignature and IsBuiltIn
    F->>F: intern safe structural node
  end
  F-->>P: folded AST DAG
  P->>D: Decompose folded AST
  D->>R: verify subtree reuse safety
  D-->>G: shared ValueValidityPair/Dex graph
  G->>G: reuse Dex value only in current BasicBlock
  G->>E: FinalizeModule
  E->>E: capture optional pre-optimization IR
  E->>E: optimize and JIT
  E-->>P: compiled code
  P-->>C: Projector or Filter

Loading

Focused safety tests

The local FoldCommonSubexpressions() test suite now directly covers:

• positive sharing within one expression and across input expression objects;
• kResultNullInternal;
• NeedsContext();
• NeedsFunctionHolder();
• CanReturnErrors();
• a safe parent with an unsafe child;
• unknown functions;
• custom registered functions with default flags;
• opaque InExpressionNode;
• different literal values and types;
• operand order (add(a,b) versus add(b,a));
• Boolean algebra not being applied;
• if algebra not being applied.

IR/runtime tests additionally verify:

• two repeated add calls become one in the same block;
• repeated values are not reused across if branches;
• if(c, x, x) still contains its condition, branch blocks, and phi;
• Boolean short-circuit CFG is preserved;
• the nested repeated-between pattern still emits three copies of each comparison
  because the copies occur in different Boolean blocks;
• filter cache behavior no longer relies on Boolean deduplication;
• cached and mutable-configuration unoptimized-IR error paths.

Local test results:

Test binary	Result
gandiva-internals-test	169 passed
gandiva-projector-test with TZ=UTC	226 passed, 1 existing aarch64 skip
gandiva-precompiled-test	132 passed
git diff --check	passed

Benchmark method

• Apple M4, Release build, LLVM 20.1.7;
• Google Benchmark, three repetitions, CPU-time mean;
• expression sizes 10, 100, and 1,000;
• three patterns: deep unique, balanced repeated safe, and balanced repeated unsafe;
• measured fold-only, Projector/Filter build, unoptimized IR bytes, and evaluation
  separately;
• the disabled baseline bypassed only the AST folding calls, with identical inputs and
  LLVM settings;
• input AST construction was paused outside the timed build section.

Results at size 1,000

Pattern	Projector build, disabled -> enabled	Filter build, disabled -> enabled	Projector unoptimized IR
Deep unique	178.7 -> 186.4 ms (+4.3%)	50.4 -> 52.1 ms (+3.4%)	116,902 -> 116,902 bytes
Repeated safe	81.6 -> 21.4 ms (-73.8%)	78.5 -> 22.0 ms (-72.0%)	1,923,715 -> 5,963 bytes (-99.69%)
Repeated unsafe	4,256.4 -> 4,356.6 ms (+2.4%)	4,450.3 -> 4,487.1 ms (+0.8%)	2,350,279 -> 2,350,279 bytes

Repeated-safe scaling:

Size	Projector build	Filter build	IR reduction
10	17.1 -> 18.2 ms (+6.5%)	15.9 -> 16.0 ms (+0.5%)	-78.2%
100	20.7 -> 17.7 ms (-14.5%)	19.0 -> 16.6 ms (-12.6%)	-97.2%
1,000	81.6 -> 21.4 ms (-73.8%)	78.5 -> 22.0 ms (-72.0%)	-99.69%

Evaluation at size 1,000:

Pattern	Projector	Filter
Deep unique	254.44 -> 255.10 us (+0.3%)	433.97 -> 434.44 us (+0.1%)
Repeated safe	19.42 -> 17.24 us (-11.2%)	20.68 -> 18.97 us (-8.2%)
Repeated unsafe	525.48 -> 523.48 us (-0.4%)	530.93 -> 527.66 us (-0.6%)

The strongest result is the reduction in compiler input and build time for repeated
safe expressions. Unique expressions pay a small linear hash-consing cost, while unsafe
expressions preserve their original IR.

Before I push the follow-up, I would appreciate feedback on these boundaries:

1. Is built-in-only reuse an acceptable initial safety contract?
2. Is same-basic-block Dex reuse sufficiently conservative for the first version?
3. Should AST interning and execution-value reuse be represented as separate concepts?
4. Would explicit function effect/CSE-safety metadata be the preferred way to consider
   arena-allocating deterministic functions such as lower and upper later?