[GatherND]: Implement verification, shape inference, and code gen.

[etiotto]

Implement support for the ONNX GatherND operator:

• verification code (diagnose operator constraints)
• shape inference with helper
• codegen support
• lit tests to verify constraint diagnostics
• lit tests to verify shape inference
• add lit test to verify code generation
• enable end-to-end test (backend test)

Signed-off-by: Ettore Tiotto etiotto@ca.ibm.com

[AlexandreEichenberger]

LGTM

[AlexandreEichenberger]

@sstamenova I am seeing MLIR-Windows-CI cancelled. Would you be able to let us know what is happening? I have seen it for many other PRs as well. Thanks

[AlexandreEichenberger]

@etiotto there seems to be an issue during compilation

ile included from /workdir/onnx-mlir/src/Runtime/OMTensor.cpp:16:
/workdir/onnx-mlir/src/Runtime/OMTensor.inc: In function 'void omTensorPrint(const char*, const OMTensor*)':
/workdir/onnx-mlir/src/Runtime/OMTensor.inc:475:24: error: taking address of temporary array
  475 |     LOOP_1(((int64_t[]){i}), i, shape[0])
      |            ~~~~~~~~~~~~^~~~

[sstamenova]

@sstamenova I am seeing MLIR-Windows-CI cancelled. Would you be able to let us know what is happening? I have seen it for many other PRs as well. Thanks

There's most likely an outage that's impacting the builds - the logs don't seem to indicate that anything else is going on. :(. Let me know if it persists and I'll spend some more time looking into it, but if it's an outage, I expect it will go back to normal soon.

[etiotto]

@etiotto there seems to be an issue during compilation

ile included from /workdir/onnx-mlir/src/Runtime/OMTensor.cpp:16:
/workdir/onnx-mlir/src/Runtime/OMTensor.inc: In function 'void omTensorPrint(const char*, const OMTensor*)':
/workdir/onnx-mlir/src/Runtime/OMTensor.inc:475:24: error: taking address of temporary array
  475 |     LOOP_1(((int64_t[]){i}), i, shape[0])
      |            ~~~~~~~~~~~~^~~~

I am using compound literals (https://en.cppreference.com/w/c/language/compound_literal) which are legal constructs since C99. Likely the issue here is that OMTensor.inc is included in both OMTensor.c and OMTensor.cpp and when compiling OMTensor.cpp the C++ compiler emits a warning because unfort. compound literals aren't part of the C++ standard specification.

[AlexandreEichenberger]

It does not seems to be a warning, I see an "Error"

[etiotto]

Yes I pushed a fix now.

[etiotto]

@sstamenova I have some code in OMTensor.inc that uses function like macros. The code compiles on all platforms except on Windows. Looking at the messages in the log did not really help me figuring out what the issue might be. Could you take a look if you have some time please ? Any suggestions very much appreciated.

P.S.: I could undo the change in OMTensor.inc as it is not essential for this PR. But if the issue turn out to be easy to fix that would be preferable.

[etiotto]

@sstamenova I have some code in OMTensor.inc that uses function like macros. The code compiles on all platforms except on Windows. Looking at the messages in the log did not really help me figuring out what the issue might be. Could you take a look if you have some time please ? Any suggestions very much appreciated.

P.S.: I could undo the change in OMTensor.inc as it is not essential for this PR. But if the issue turn out to be easy to fix that would be preferable.

I got a Windows VM and was able to reproduce the issue using a cut down test. It turns out that MSVC, for historical reasons, "interprets" the C specification somewhat differently than gcc & clang. MSCV provides a compiler option (https://docs.microsoft.com/en-us/cpp/preprocessor/preprocessor-experimental-overview?view=msvc-170) that makes the preprocessor more conforming to the C standard. Using that flag fixed the problem for the reduced test case. I have added it to the CMakefile and triggered a new build ...

[sstamenova]

@sstamenova I have some code in OMTensor.inc that uses function like macros. The code compiles on all platforms except on Windows. Looking at the messages in the log did not really help me figuring out what the issue might be. Could you take a look if you have some time please ? Any suggestions very much appreciated.
P.S.: I could undo the change in OMTensor.inc as it is not essential for this PR. But if the issue turn out to be easy to fix that would be preferable.

I got a Windows VM and was able to reproduce the issue using a cut down test. It turns out that MSVC, for historical reasons, "interprets" the C specification somewhat differently than gcc & clang. MSCV provides a compiler option (https://docs.microsoft.com/en-us/cpp/preprocessor/preprocessor-experimental-overview?view=msvc-170) that makes the preprocessor more conforming to the C standard. Using that flag fixed the problem for the reduced test case. I have added it to the CMakefile and triggered a new build ...

@etiotto : Please let me know if it doesn't work and I'll have a look. I was OOF earlier this week, but I'm back in the office now.

[jenkins-droid]

Jenkins Linux s390x Build #5599 [push] [GatherND]: Implement v... started at 13:00

[jenkins-droid]

Jenkins Linux amd64 Build #5584 [push] [GatherND]: Implement v... started at 12:00

[jenkins-droid]

Jenkins Linux ppc64le Build #4721 [push] [GatherND]: Implement v... started at 13:02

[jenkins-droid]

Jenkins Linux s390x Build #5599 [push] [GatherND]: Implement v... passed after 53 min

[jenkins-droid]

Jenkins Linux ppc64le Build #4721 [push] [GatherND]: Implement v... passed after 57 min

[jenkins-droid]

Jenkins Linux amd64 Build #5584 [push] [GatherND]: Implement v... passed after 1 hr 2 min