Skip to content

tests: normalize LLVM IR when updating goldens - #5574

Open
jakebailey wants to merge 1 commit into
tinygo-org:devfrom
jakebailey:normalize-ir
Open

tests: normalize LLVM IR when updating goldens#5574
jakebailey wants to merge 1 commit into
tinygo-org:devfrom
jakebailey:normalize-ir

Conversation

@jakebailey

Copy link
Copy Markdown
Member

This bugged me while working on another branch, as -update kept overwriting all of my golden files with LLVM 22 outputs instead of the normalized ones. I think it's better to just store it normalized.

@jakebailey
jakebailey requested review from dgryski and a lite review from Copilot August 7, 2026 05:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the LLVM IR golden-file workflow so regenerated goldens are stored in a normalized form (to reduce LLVM-version churn) and updates an existing golden accordingly.

Changes:

  • Update -update paths in transform/compiler tests to avoid unnecessary golden rewrites when outputs are equivalent across LLVM versions.
  • Normalize LLVM-version-specific IR spellings before writing goldens.
  • Refresh compiler/testdata/large.ll to the normalized (pre-LLVM21-style) attribute spellings.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
transform/transform_test.go Adjusts golden update logic for transform tests and applies IR normalization when writing .out.ll files.
compiler/compiler_test.go Adjusts golden update logic for compiler tests and normalizes IR when writing .ll goldens; also refactors comparison to reuse actual.
compiler/testdata/large.ll Updates a golden file to use normalized IR spellings (e.g., nocapture instead of captures(none)).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread compiler/compiler_test.go Outdated
Comment thread transform/transform_test.go Outdated

@dgryski dgryski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. At some point we should update the version of LLVM that we're normalizing to.

@jakebailey
jakebailey requested a lite review from Copilot August 7, 2026 16:21
@jakebailey

Copy link
Copy Markdown
Member Author

The repo's llvm-project clone is 22 so arguably that should be the one being used...

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

@dgryski

dgryski commented Aug 7, 2026

Copy link
Copy Markdown
Member

The various regexps in normalizeIR rewrite to pre-LLVM22 versions though (captures(none) -> nocapture, etc)

@jakebailey

Copy link
Copy Markdown
Member Author

Right, I didn't change the status quo in this PR, I was just suggesting 22 for some unspecified future date (which could be now if desired?)

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.

3 participants