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.