This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Affine] Add ValueBoundsOpInterface for affine.apply
ClosedPublic

Authored by springerm on Mar 9 2023, 8:12 AM.

Diff Detail

Event Timeline

springerm created this revision.Mar 9 2023, 8:12 AM
springerm requested review of this revision.Mar 9 2023, 8:12 AM
springerm retitled this revision from [mlir][linalg] ValueBoundsOpInterface: Add affine.apply op to [mlir][Affine] Add ValueBoundsOpInterface for affine.apply .Mar 16 2023, 8:15 AM
springerm updated this revision to Diff 506069.Mar 17 2023, 6:36 AM
springerm retitled this revision from [mlir][Affine] Add ValueBoundsOpInterface for affine.apply to [mlir][Affine] Add ValueBoundsOpInterface for affine.apply.

rebase

springerm updated this revision to Diff 509963.Mar 31 2023, 3:28 AM

update API

dcaballe added inline comments.Mar 31 2023, 12:10 PM
mlir/lib/Dialect/Affine/IR/ValueBoundsOpInterfaceImpl.cpp
37

== -> =? Add a test for this case if it's not covered?

mlir/test/Dialect/Affine/value-bounds-op-interface-impl.mlir
8

Use different var names?

dcaballe accepted this revision.Mar 31 2023, 12:18 PM
dcaballe added inline comments.
mlir/lib/Dialect/Affine/IR/ValueBoundsOpInterfaceImpl.cpp
37

nevermind :)

mlir/test/Dialect/Affine/value-bounds-op-interface-impl.mlir
8

nevermind

This revision is now accepted and ready to land.Mar 31 2023, 12:18 PM

@springerm The test suite is failing due to order stability issues related to these changes (or other revisions related to this):

******************** TEST 'MLIR :: Dialect/Tensor/value-bounds-op-interface-impl.mlir' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/uday/llvm-project-upstream/build/bin/mlir-opt /home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir -test-affine-reify-value-bounds -verify-diagnostics      -split-input-file | /home/uday/llvm-project-upstream/build/bin/FileCheck /home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir
--
Exit Code: 1

Command Output (stderr):
--
/home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir:107:11: error: CHECK: expected string not found in input
// CHECK: #[[$map:.*]] = affine_map<()[s0, s1] -> (s0 + s1 * 2)>
          ^
<stdin>:70:13: note: scanning from here
 return %dim : index
            ^
<stdin>:76:1: note: possible intended match here
#map = affine_map<()[s0, s1] -> (s0 * 2 + s1)>
^
/home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir:113:44: error: undefined variable: $map
// CHECK: %[[bound0:.*]] = affine.apply #[[$map]]()[%[[dim0]], %[[a]]]

@springerm The test suite is failing due to order stability issues related to these changes (or other revisions related to this):

******************** TEST 'MLIR :: Dialect/Tensor/value-bounds-op-interface-impl.mlir' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/uday/llvm-project-upstream/build/bin/mlir-opt /home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir -test-affine-reify-value-bounds -verify-diagnostics      -split-input-file | /home/uday/llvm-project-upstream/build/bin/FileCheck /home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir
--
Exit Code: 1

Command Output (stderr):
--
/home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir:107:11: error: CHECK: expected string not found in input
// CHECK: #[[$map:.*]] = affine_map<()[s0, s1] -> (s0 + s1 * 2)>
          ^
<stdin>:70:13: note: scanning from here
 return %dim : index
            ^
<stdin>:76:1: note: possible intended match here
#map = affine_map<()[s0, s1] -> (s0 * 2 + s1)>
^
/home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir:113:44: error: undefined variable: $map
// CHECK: %[[bound0:.*]] = affine.apply #[[$map]]()[%[[dim0]], %[[a]]]

Do you have any extra changes in your build? Does it only happen sometimes or every time you run the test? I cannot reproduce this locally (ran the test 100 times without failure), also no build bot failure email so far.

bondhugula reopened this revision.EditedApr 6 2023, 5:07 AM

@springerm The test suite is failing due to order stability issues related to these changes (or other revisions related to this):

******************** TEST 'MLIR :: Dialect/Tensor/value-bounds-op-interface-impl.mlir' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/uday/llvm-project-upstream/build/bin/mlir-opt /home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir -test-affine-reify-value-bounds -verify-diagnostics      -split-input-file | /home/uday/llvm-project-upstream/build/bin/FileCheck /home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir
--
Exit Code: 1

Command Output (stderr):
--
/home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir:107:11: error: CHECK: expected string not found in input
// CHECK: #[[$map:.*]] = affine_map<()[s0, s1] -> (s0 + s1 * 2)>
          ^
<stdin>:70:13: note: scanning from here
 return %dim : index
            ^
<stdin>:76:1: note: possible intended match here
#map = affine_map<()[s0, s1] -> (s0 * 2 + s1)>
^
/home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir:113:44: error: undefined variable: $map
// CHECK: %[[bound0:.*]] = affine.apply #[[$map]]()[%[[dim0]], %[[a]]]

Do you have any extra changes in your build? Does it only happen sometimes or every time you run the test? I cannot reproduce this locally (ran the test 100 times without failure), also no build bot failure email so far.

I have no local changes - it happens on the upstream main branch all the time. We have an unstable container or an order of evaluation dependent code somewhere.

This revision is now accepted and ready to land.Apr 6 2023, 5:07 AM

@springerm The test suite is failing due to order stability issues related to these changes (or other revisions related to this):

******************** TEST 'MLIR :: Dialect/Tensor/value-bounds-op-interface-impl.mlir' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/uday/llvm-project-upstream/build/bin/mlir-opt /home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir -test-affine-reify-value-bounds -verify-diagnostics      -split-input-file | /home/uday/llvm-project-upstream/build/bin/FileCheck /home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir
--
Exit Code: 1

Command Output (stderr):
--
/home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir:107:11: error: CHECK: expected string not found in input
// CHECK: #[[$map:.*]] = affine_map<()[s0, s1] -> (s0 + s1 * 2)>
          ^
<stdin>:70:13: note: scanning from here
 return %dim : index
            ^
<stdin>:76:1: note: possible intended match here
#map = affine_map<()[s0, s1] -> (s0 * 2 + s1)>
^
/home/uday/llvm-project-upstream/mlir/test/Dialect/Tensor/value-bounds-op-interface-impl.mlir:113:44: error: undefined variable: $map
// CHECK: %[[bound0:.*]] = affine.apply #[[$map]]()[%[[dim0]], %[[a]]]

Do you have any extra changes in your build? Does it only happen sometimes or every time you run the test? I cannot reproduce this locally (ran the test 100 times without failure), also no build bot failure email so far.

I have no local changes - it happens on the upstream main branch all the time. We have an unstable container or an order of evaluation dependent code somewhere.

This appears to be due to an order of evaluation issue: IR being generated with no guaranteed order. Happens with GCC 8.5.0, GCC 9.4.0, at least. Are you using clang?

This appears to be due to an order of evaluation issue: IR being generated with no guaranteed order. Happens with GCC 8.5.0, GCC 9.4.0, at least. Are you using clang?

I found the problem. This happens with gcc only.

The problematic code is:

AffineExpr expr = cstr.getExpr(padOp.getSource(), dim) +
                  cstr.getExpr(padOp.getMixedLowPad()[dim]) +
                  cstr.getExpr(padOp.getMixedHighPad()[dim]);

The evaluation order of the operands in the sum is unspecified in C++. clang evaluates left to right. gcc evaluates right to left. getExpr has a side effect where things a put on a worklist. The order in which things are put on the worklist determines the order in which columns are projected out in the FlatLinearConstraints. Different orders of projection produce different affine_maps during getSliceBounds.

The easy fix here is to split the above statement into 4 statements. But I'm wondering if there is a better to fix this. I think this won't be the last time that we're seeing this issue.

bondhugula added a comment.EditedApr 7 2023, 6:15 PM

This is the right fix. We've seen such things in the past. Things generating IR can't be combined in a expression that way. Another common problematic pattern is:
... = b.create<...>(loc, b.create<....>(...), b.create<....>(...));. One can't guarantee here in what order IR will be generated. In general, whenever you have multiple expressions being combined, the individual expressions shouldn't generate IR.

springerm closed this revision.Apr 10 2023, 6:51 PM