Skip to content

perf: detect unnecessary repeated checks in more situations#11078

Closed
hargoniX wants to merge 3 commits intomasterfrom
hbv/even_less_double_checks
Closed

perf: detect unnecessary repeated checks in more situations#11078
hargoniX wants to merge 3 commits intomasterfrom
hbv/even_less_double_checks

Conversation

@hargoniX
Copy link
Contributor

@hargoniX hargoniX commented Nov 4, 2025

This PR improves on #11020 to remove even more unnecessary checks

@hargoniX hargoniX requested a review from leodemoura as a code owner November 4, 2025 15:38
@hargoniX hargoniX added the changelog-compiler Compiler, runtime, and FFI label Nov 4, 2025
@hargoniX
Copy link
Contributor Author

hargoniX commented Nov 4, 2025

!radar

@leanprover-radar
Copy link

leanprover-radar commented Nov 4, 2025

Benchmark results for 1353298 against e76bbef are in! @hargoniX

Runs (2)
  • other exited with code 2 (🟥)
  • stdlib exited with code 2 (🟥)

@hargoniX
Copy link
Contributor Author

hargoniX commented Nov 4, 2025

Well clearly that's no good, time to investigate after vacation :D

@hargoniX
Copy link
Contributor Author

!radar

@leanprover-radar
Copy link

leanprover-radar commented Nov 17, 2025

Benchmark results for cc72ace against 27e5e21 are in! @hargoniX

Major changes (2)
  • liasolver//instructions changed by +1.0% (🟥).
  • simp_local//instructions changed by +1.4% (🟥).
Minor changes (12)
  • bv_decide_rewriter.lean//instructions changed by -0.8% (✅).
  • ilean roundtrip//instructions changed by +0.9% (🟥).
  • lake build no-op//instructions changed by +0.6% (🟥).
  • libleanshared.so//binary size changed by +0.6% (🟥).
  • parser//instructions changed by -0.7% (✅).
  • simp_bubblesort_256//instructions changed by +0.9% (🟥).
  • simp_subexpr//instructions changed by +0.9% (🟥).
  • stdlib size//bytes .ir changed by +0.4% (🟥).
  • stdlib size//bytes .olean.private changed by -0.5% (✅).
  • stdlib size//lines C changed by +0.3% (🟥).
  • stdlib//instructions changed by -0.2% (✅).
  • tests/compiler//sum binary sizes changed by +0.5% (🟥).

github-merge-queue bot pushed a commit that referenced this pull request Nov 17, 2025
This PR fixes a bug in the LCNF simplifier unearthed while working on
#11078. In some situations caused by `unsafeCast`, the simplifier would
record incorrect information about `cases`, leading to further bugs down
the line.

Suppose we have `v : NonScalar` due to an `unsafeCast` and we run
`cases` on it, expecting `Prod.mk fst snd`. The current code attempts to
record both the arguments from the constructor application in the case
arm `fst`, `snd` and the parameters for the type by inspecting the discr
`v`. However, `NonScalar` does of course not have any parameters,
causing the simplifier to record wrong information. This patch makes the
`cases` infrastructure more cautious when extracting information from
the type of `v`.
@hargoniX hargoniX force-pushed the hbv/even_less_double_checks branch from cc72ace to a27d665 Compare November 17, 2025 12:08
@hargoniX
Copy link
Contributor Author

!radar

@leanprover-radar
Copy link

leanprover-radar commented Nov 17, 2025

Benchmark results for a27d665 against bef8574 are in! @hargoniX

Major changes (2)
  • liasolver//instructions changed by +1.0% (🟥).
  • simp_local//instructions changed by +1.4% (🟥).
Minor changes (13)
  • bv_decide_realworld//instructions changed by -0.5% (✅).
  • bv_decide_rewriter.lean//instructions changed by -0.8% (✅).
  • ilean roundtrip//instructions changed by +0.9% (🟥).
  • lake build no-op//instructions changed by +0.6% (🟥).
  • libleanshared.so//binary size changed by +0.6% (🟥).
  • parser//instructions changed by -0.7% (✅).
  • simp_bubblesort_256//instructions changed by +1.0% (🟥).
  • simp_subexpr//instructions changed by +0.9% (🟥).
  • stdlib size//bytes .ir changed by +0.4% (🟥).
  • stdlib size//bytes .olean.private changed by -0.5% (✅).
  • stdlib size//lines C changed by +0.3% (🟥).
  • stdlib//instructions changed by -0.2% (✅).
  • tests/compiler//sum binary sizes changed by +0.5% (🟥).

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Nov 17, 2025
@ghost
Copy link

ghost commented Nov 17, 2025

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase bef8574b93cacb87c7e9804870341441346a64b5 --onto 8b575dcbf2a3b907044df2233a61b61003eeeb45. You can force Mathlib CI using the force-mathlib-ci label. (2025-11-17 13:43:45)

@leanprover-bot
Copy link
Collaborator

Reference manual CI status:

  • ❗ Reference manual CI will not be attempted unless your PR branches off the nightly-with-manual branch. Try git rebase bef8574b93cacb87c7e9804870341441346a64b5 --onto d3dda9f6d4428a906c096067ecb75e432afc4615. You can force reference manual CI using the force-manual-ci label. (2025-11-17 13:43:46)

@hargoniX
Copy link
Contributor Author

Seems like this breaks reuse more than it helps :(

@hargoniX hargoniX closed this Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-compiler Compiler, runtime, and FFI toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants