Skip to content

25662: Adds simplify opcode, improves consistency of subtraction and division opcodes, MINOR - #634

Merged
howsohazard merged 85 commits into
mainfrom
25662-simplify-opcode2
Aug 12, 2026
Merged

25662: Adds simplify opcode, improves consistency of subtraction and division opcodes, MINOR#634
howsohazard merged 85 commits into
mainfrom
25662-simplify-opcode2

Conversation

@howsohazard

@howsohazard howsohazard commented Aug 10, 2026

Copy link
Copy Markdown
Contributor
  • Adds simplify opcode that performs basic simplifications
  • Unitary division now performs reciprocal analogous to unitary minus, and pow now supports unitary operation like addition and multiplication
  • Fixes numerous consistency issues regarding code mixing and merging, especially involving multiplication and division
  • Enhances flexibility of a number of math opcodes

@howso-mario howso-mario left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes on current head 1a840713 after a fresh full-diff review against current main. I found three focused issues in the new simplify/sort behavior; the inline comments include exact reproducers and patch/test shapes.

Verification: the CI-faithful amd64-release-linux-228 configure/build/test completed all 472 build steps and passed 13/13 CTest smoke targets. Remote PR checks are also green across all product builds/smoke tests/docs regeneration. Focused runtime probes on that build nevertheless returned "adXc" for an ordered concat whose original result is "aXcd", rewrote (/ a 49) to (/ a 49.00000000000001), and returned 0.9999999999999999 for the corresponding a=49 call. The zero-arity sorted-unparse case happens to print successfully in this release build, but constructs an invalid random-access iterator range and is undefined behavior.

I inventoried all six historical threads before reviewing current HEAD. The null-result crash, boolean absorbing value, distinguished subtraction/division operand/arity, and generic Merger<T> issues are fixed and are not restated. The prior division thread is only partially superseded because the reciprocal round-trip below remains. Two independent review lanes (Claude Code primary, Codex second-opinion) agreed on the ordered-concat and invalid-sort-range blockers; both also identified the reciprocal canonicalization defect. No other blocking findings remain from this pass.

Comment thread src/Amalgam/evaluablenode/EvaluableNodeTreeAlgebra.cpp Outdated
Comment thread src/Amalgam/Parser.cpp Outdated
Comment thread src/Amalgam/evaluablenode/EvaluableNodeTreeAlgebra.cpp Outdated

@howso-mario howso-mario left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES_REQUESTED at 6474cece. CI-faithful verification passed: ghcr.io/howsoai/amalgam-build-container-linux-228:2.0.9, preset amd64-release-linux-228, configure + full 472-target build + CTest 13/13. Live GitHub checks are 19 successful and 5 expected release-only skips. Focused probes on ST, ST-PGC, ST-AFMI, MT, and MT-AFMI reproduced the semantic differences in the inline blockers.

Comment thread src/Amalgam/evaluablenode/EvaluableNodeTreeAlgebra.cpp
Comment thread src/Amalgam/evaluablenode/EvaluableNodeTreeAlgebra.cpp Outdated
Comment thread src/Amalgam/evaluablenode/EvaluableNodeTreeAlgebra.cpp Outdated
Comment thread src/Amalgam/evaluablenode/EvaluableNodeTreeAlgebra.cpp

@howso-mario howso-mario left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed current head 39a12b1 against the previously verified bd5e2ae head. The four findings from review 4912005498 remain resolved: zero-divisor ordering is preserved, unsafe exp/log inverses remain removed, symbolic subtraction/division heads are not cancelled, and node-type mutation paths return before reusing invalidated child vectors. The post-review delta is limited to comment cleanup, a clean merge of main’s C++20/build update, and consistent Visual C++ stdcpp20 settings. All current required CI checks, including Windows, GCC 14 Linux/glibc 2.28, ARM, macOS, WASM, docs, smoke tests, and final-check, are green; release-only jobs are skipped as expected.

@howsohazard
howsohazard merged commit 2391010 into main Aug 12, 2026
24 checks passed
@howsohazard
howsohazard deleted the 25662-simplify-opcode2 branch August 12, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants