Skip to content

Conversation

@frederik-h
Copy link
Contributor

@frederik-h frederik-h commented Oct 13, 2025

The last change to the pass in PR #158588 lost the assignment to the "Modified" variable for one of the pass optimizations.

Add it back. This fixes the test failure in CodeGen/AMDGPU/itofp.i128.bf.ll (in a LLVM_ENABLE_EXPENSIVE_CHECKS=ON build).

The last change to the pass lost the assignment to the
"Modified" variable for one of the pass optimizations.
Add it back.
@frederik-h
Copy link
Contributor Author

This fixes the test failure in CodeGen/AMDGPU/itofp.i128.bf.ll (in a LLVM_ENABLE_EXPENSIVE_CHECKS=ON build) that was caused by PR #158588 and reported by the build bots.

continue;

addToWorklist(I, Worklist);
Modified = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about removing all the assignments on add to worklist, and do it once when processing the worklist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is possible provided scalarize always pushes to the Worklist, i.e. the instructions created in this function cannot be folded. This seems to hold true for the instructions handled in this pass. I have implemented the change accordingly in this commit. Does that make sense or should I perhaps pass NoFolder to the IRBuilder ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll revert the additional changes for now to fix the build bot failures. We can still add them in a follow-up PR.

@frederik-h frederik-h requested a review from RKSimon October 13, 2025 14:50
Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

Thanks - fixes my local checks

@frederik-h frederik-h enabled auto-merge (squash) October 13, 2025 15:06
@frederik-h frederik-h merged commit 8cc862c into llvm:main Oct 13, 2025
9 of 10 checks passed
akadutta pushed a commit to akadutta/llvm-project that referenced this pull request Oct 14, 2025
The last change to the pass in PR llvm#158588 lost the assignment to the
"Modified" variable for one of the pass optimizations.

Add it back. This fixes the test failure in
`CodeGen/AMDGPU/itofp.i128.bf.ll` (in a
`LLVM_ENABLE_EXPENSIVE_CHECKS=ON` build).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants