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

Dynamic backend tests failure in IndexExpr #2755

Open
mikeessen opened this issue Mar 14, 2024 · 2 comments
Open

Dynamic backend tests failure in IndexExpr #2755

mikeessen opened this issue Mar 14, 2024 · 2 comments

Comments

@mikeessen
Copy link
Collaborator

Backend dynamic tests for Pad (test_constant_pad_cpu, test_edge_pad_cpu, and test_reflect_pad_cpu) fail when the dynamic shape is -1:-1 for testing all inputs.

Assertion failed: (indexExprObj), function getObjPtr, file IndexExpr.cpp, line 439.

The tests pass if src/Dialect/Mlir/IndexExprBuilder.cpp line 200 the following condition is removed:

  if (size == ShapedType::kDynamic || i >= (uint64_t)size) {
    return UndefinedIndexExpr();
  }

These tests also pass if the dynamic shape is 0:-1. PR #2754 was delivered to to update the dynamic shape to 0:-1.

@mikeessen
Copy link
Collaborator Author

@AlexandreEichenberger is the test condition in IndexExprBuilder.cpp noted above appropriate given the dynamic tensor sizes?

@AlexandreEichenberger
Copy link
Collaborator

AlexandreEichenberger commented Mar 18, 2024

Typically not, unless it is very controlled.

Basically, the code is trying to get a value from an array for an index I when we don't know the array size. Say array is defined as a:[?xi64] and we are asking to get a[10]. The compiler doesn't know if the array has 11 values. So up to now, we only allowed a call to this method when the array was of known size.

But if you were to look at the code, and it is really like this (aka careful code)

%dim = dim %a: type<?xi64>
for %I =0; %I<%dim, %I++) {
  %v = "getValFromArray(%a, %i)

or you know that another parameter to the operation that you are lowering, say %axis must be by definition in the range 0 ... %dim even if that dim is only known at runtime.

then maybe the code might work, but I would have to look at the generated code much more carefully.

So if you are in that later situation, you could try to add a flag dynamicArraySizeAllowed, enable it for this case, and see what the output is.

In addition, if the current "user" of that call expected a constant value, then the code will also not work. Aka if the algorithm expected to know %a[10] == 3 and then use that value 3 explicitly, now having an IndexExpr = "runtime value" would not work. Namely, the algorithm using that value would have to be rewritten to accommodate a value only known at runtime.

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

No branches or pull requests

2 participants