Skip to content

llvm-reduce: Add reduceOperandsToPoison reduction #132862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2025

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Mar 25, 2025

For now use it only for TargetExtTypes, which do not always support
zero initializers.

Copy link
Contributor Author

arsenm commented Mar 25, 2025

@arsenm arsenm marked this pull request as ready for review March 25, 2025 02:31
@arsenm arsenm force-pushed the users/arsenm/llvm-reduce/fix-assert-operands-to-zero-target-ext-type branch from 2eab5bb to 633b42c Compare March 25, 2025 04:35
Base automatically changed from users/arsenm/llvm-reduce/fix-assert-operands-to-zero-target-ext-type to main March 25, 2025 04:38
@arsenm arsenm force-pushed the users/arsenm/llvm-reduce/add-reduce-operands-poison branch from 7271ac9 to e40a40c Compare March 25, 2025 04:42
@regehr
Copy link
Contributor

regehr commented Mar 25, 2025

I'm not personally a fan of reducing to poison. this can make test cases smaller but the cognitive load of understanding poison is significant, so I'm not sure this is really a win

@arsenm
Copy link
Contributor Author

arsenm commented Mar 25, 2025

I'm not personally a fan of reducing to poison. this can make test cases smaller but the cognitive load of understanding poison is significant, so I'm not sure this is really a win

The alternative for target extension types that don't support zero initializer is to do nothing, which is not a win

@arsenm arsenm force-pushed the users/arsenm/llvm-reduce/add-reduce-operands-poison branch from e40a40c to 705dd28 Compare March 28, 2025 07:12
@arsenm arsenm force-pushed the users/arsenm/llvm-reduce/add-reduce-operands-poison branch from 705dd28 to 7e067d4 Compare March 30, 2025 10:46
@@ -168,3 +166,18 @@ void llvm::reduceOperandsNaNDeltaPass(Oracle &O, ReducerWorkItem &WorkItem) {
};
extractOperandsFromModule(O, WorkItem, ReduceValue);
}

void llvm::reduceOperandsPoisonDeltaPass(Oracle &O, ReducerWorkItem &WorkItem) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a comment that we're only doing this for non-constant TargetExtTypes?

Copy link
Contributor Author

arsenm commented Apr 2, 2025

Merge activity

  • Apr 2, 2:34 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 2, 2:36 AM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 2, 2:38 AM EDT: A user merged this pull request with Graphite.

For now use it only for TargetExtTypes, which do not always support
zero initializers.
@arsenm arsenm force-pushed the users/arsenm/llvm-reduce/add-reduce-operands-poison branch from 7e067d4 to 0d4807e Compare April 2, 2025 06:36
@arsenm arsenm merged commit 964650b into main Apr 2, 2025
6 of 9 checks passed
@arsenm arsenm deleted the users/arsenm/llvm-reduce/add-reduce-operands-poison branch April 2, 2025 06:38
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.

3 participants