Skip to content
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

[mlir][Interfaces] ValueBoundsOpInterface: Fix typo #87976

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

matthias-springer
Copy link
Member

This was likely a copy-and-paste typo.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 8, 2024

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

This was likely a copy-and-paste typo.


Full diff: https://github.com/llvm/llvm-project/pull/87976.diff

1 Files Affected:

  • (modified) mlir/lib/Interfaces/ValueBoundsOpInterface.cpp (+1-1)
diff --git a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
index 1f3f1bb863c554..05b65f7460edcc 100644
--- a/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
+++ b/mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
@@ -205,7 +205,7 @@ int64_t ValueBoundsConstraintSet::insert(bool isSymbol) {
 int64_t ValueBoundsConstraintSet::insert(AffineMap map, ValueDimList operands,
                                          bool isSymbol) {
   assert(map.getNumResults() == 1 && "expected affine map with one result");
-  int64_t pos = insert(/*isSymbol=*/false);
+  int64_t pos = insert(isSymbol);
 
   // Add map and operands to the constraint set. Dimensions are converted to
   // symbols. All operands are added to the worklist (unless they were already

@joker-eph
Copy link
Collaborator

Can you provide a test that exercises this? Thanks!

@matthias-springer
Copy link
Member Author

matthias-springer commented Apr 8, 2024

It is currently not possible to trigger this bug. New columns are always added to the constraint set as symbols. Only the first added SSA value (for which we compute a bound) is added as a dimension. There is currently no API to compute a bound for an "AffineMap + operands" combination. (But in anticipation of adding such an API in the future and for consistency reasons, I had already added an isSymbol argument to all ValueBoundsConstraintSet::insert overloads.) I have a PR in preparation that extends the API and will require this fix here.

@matthias-springer matthias-springer force-pushed the users/matthias-springer/value_bounds_compare branch from 62fe9bb to 772389c Compare April 8, 2024 11:21
@matthias-springer matthias-springer force-pushed the users/matthias-springer/value_bounds_typo branch from a09cbb2 to 4a019ca Compare April 8, 2024 11:22
@matthias-springer matthias-springer force-pushed the users/matthias-springer/value_bounds_compare branch from 772389c to 80949fe Compare April 8, 2024 13:51
@matthias-springer matthias-springer force-pushed the users/matthias-springer/value_bounds_typo branch from 4a019ca to d8c3fd6 Compare April 8, 2024 13:52
Base automatically changed from users/matthias-springer/value_bounds_compare to main April 11, 2024 06:23
This was likely a copy-and-paste typo.
@matthias-springer matthias-springer force-pushed the users/matthias-springer/value_bounds_typo branch from d8c3fd6 to 0a7a53e Compare April 11, 2024 06:25
@matthias-springer matthias-springer merged commit 21265f6 into main Apr 11, 2024
3 of 4 checks passed
@matthias-springer matthias-springer deleted the users/matthias-springer/value_bounds_typo branch April 11, 2024 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants