[NNPA] An option to report why the operations are not run on NNPA (--opt-report=NNPAUnsupportedOps) #2819

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.

Sign up for GitHub

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

Jump to bottom

Merged

imaihal merged 35 commits into onnx:main from imaihal:legality_check_msg

Jun 6, 2024

Collaborator

imaihal commented May 8, 2024 •

edited

Loading

Generate report on why the operations are not run on NNPA.
The report is displayed by using --opt-report=NNPAUnsupportedOps. The format is
==NNPA-UNSUPPORTEDOPS-REPORT==, <operation name>, <node name>, <reason>

  --opt-report=<value>                                                              - Provide information on a specific compiler optimization.
    =NoReport                                                                       -   No report. Default value.
    =Parallel                                                                       -   Provide report on how OMP Parallel is applied to ONNX ops.
    =Simd                                                                           -   Provide report on how SIMD is applied to ONNX ops.
    =NNPAUnsupportedOps

Example of xlm-roberta-base-language-detection-onnx (issue #2813):

$ onnx-mlir --O3 --mtriple=s390x-ibm-loz --mcpu=z16 --maccel=NNPA model.onnx  --EmitZHighIR --opt-report=NNPAUnsupportedOps

==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Mul, Mul_28, Element type is not F16 or F32.
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Add, Add_31, Element type is not F16 or F32.
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Div, Div_122, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Add, Add_125, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Mul, Mul_128, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Div, Div_216, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Add, Add_219, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Mul, Mul_222, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Div, Div_310, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Add, Add_313, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Mul, Mul_316, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Div, Div_404, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Add, Add_407, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].
==NNPA-UNSUPPORTEDOPS-REPORT==, onnx.Mul, Mul_410, Rank 0 is not supported. zAIU only supports rank in range of (0, 4].


:
:

imaihal added 3 commits

May 8, 2024 15:05


          Generate messages for legality check for MatMul op.

9fac96d

Signed-off-by: Haruki Imai <[email protected]>


          format

e98741b

Signed-off-by: Haruki Imai <[email protected]>


          format

66b0046

Signed-off-by: Haruki Imai <[email protected]>

imaihal requested review from cjvolzka and tungld

May 8, 2024 15:21

tungld approved these changes

View reviewed changes

Collaborator

tungld left a comment

LGTM! It's useful. Thanks for adding this!

src/Accelerators/NNPA/Conversion/ONNXToZHigh/ONNXLegalityCheck.cpp Outdated

+              void emitLegalityCheckMessage(Operation *op, StringRef message) {
+                LLVM_DEBUG(llvm::outs() << "[NNPA Legality Check] Warning: " << op->getLoc()
+                                        << " runs on CPU. Reason: " << message << "\n");
+              }

Collaborator

tungld May 9, 2024

Could you print out the operation name also? Sometime the loc is just a number and it's not easy to know which operation is.

Collaborator Author

imaihal May 9, 2024

Added operation name and node name. Thanks!

imaihal added 3 commits

May 9, 2024 07:27


          Update messages.

92d6451

Signed-off-by: Haruki Imai <[email protected]>


          Add op name and node name in the message.

Signed-off-by: Haruki Imai <[email protected]>


          Merge branch 'main' into legality_check_msg

4f5d81f

tungld reviewed

View reviewed changes

src/Accelerators/NNPA/Conversion/ONNXToZHigh/ONNXLegalityCheck.cpp Outdated

+                    bool returnVal = (shapeA[1] == shapeB[0]);
+                    if (!returnVal)
+                      emitLegalityCheckMessage(
+                          op.getOperation(), "Unstacked case, dim A 1 not equal dim B 0.");

Collaborator

tungld May 10, 2024

Since we know the dimension size in this case, you can print out shapeA[1] and shapeB[0] as well, say Unstacked case, the 2nd dim of A (5) and the 1st dim of B (7) are not the same. if shapeA[1] =5 and shapeB[0] = 7`. Same for other static-shape cases.

Collaborator Author

imaihal May 13, 2024

Done. Thanks!

imaihal added 4 commits

May 10, 2024 08:19


          Merge branch 'main' into legality_check_msg

c83cebb


          Adde messages for NNPA compatibility.

74b8cec

Signed-off-by: Haruki Imai <[email protected]>


          Messages for verifying element type and rank.

64683b5

Signed-off-by: Haruki Imai <[email protected]>


          Added all error messages for MatMul.

5b6b63a

Signed-off-by: Haruki Imai <[email protected]>

tungld reviewed

View reviewed changes

src/Accelerators/NNPA/Conversion/ONNXToZHigh/ONNXLegalityCheck.cpp Outdated

+                  } else {
+                    std::string message = "Element type is not F16 or F32.";
+                    emitLegalityCheckMessage(op, StringRef(message));
+                    return false;

Collaborator

tungld May 13, 2024

I recommend to change emitLegalityCheckMessage to return false, e.g bool emitLegalityCheckMessage(Operation *op, StringRef message), and then we can write:

return emitLegalityCheckMessage(op, StringRef(message));

Collaborator Author

imaihal May 14, 2024

Done. Thanks. Also changed the function name.

imaihal added 3 commits

May 14, 2024 04:11


          Update messages for rewriting for onnx-to-zhigh pass.

6541f3b

Signed-off-by: Haruki Imai <[email protected]>


          Merge branch 'main' into legality_check_msg

4fa1c9c


          Add return value for function to emit messages.

b0822da

Signed-off-by: Haruki Imai <[email protected]>

tungld reviewed

View reviewed changes

src/Accelerators/NNPA/Conversion/ONNXToZHigh/ONNXLegalityCheck.cpp Outdated

-                    return ((rank > 0) && (rank <= 4));
+                    if ((rank > 0) && (rank <= 4)) {
+                      return true;
+                    } else {

Collaborator

tungld May 15, 2024

Since we have return in the if part, the else is not necessary. For example:

if ((rank > 0) && (rank <= 4))
     return true;

std::string message =
            "Rank " + std::to_string(rank) +
            " is not supported. zAIU only supports rank in range of (0, 4].";
return emitWarningMessageNNPAUnsupported(op, StringRef(message));

Collaborator Author

imaihal May 21, 2024

Done. Thanks!

imaihal added 2 commits

May 16, 2024 02:01


          Add warning messages for pooling ops.

0b3b4ba

Signed-off-by: Haruki Imai <[email protected]>


          Merge branch 'main' into legality_check_msg

a30cef2

tungld reviewed

View reviewed changes

src/Accelerators/NNPA/Conversion/ONNXToZHigh/ONNXLegalityCheck.cpp Outdated

                   }
                 }
                 return true;
               }
+              bool emitWarningMessageNNPAUnsupported(Operation *op, StringRef message) {

Collaborator

tungld May 16, 2024

Do we really need to use StringRef instead of just const std::string&? I don't see any operation on message and passing a pointer looks faster than constructing a StringRef.

Collaborator Author

imaihal May 21, 2024

Updated. Thanks!

imaihal added 5 commits

May 16, 2024 06:24


          Use std::string for the function. Add more messages for other ops.

b0d7ca9

Signed-off-by: Haruki Imai <[email protected]>


          Update messages for pooling.

86eed99

Signed-off-by: Haruki Imai <[email protected]>


          Add warning message for reduceMean.

d9284eb

Signed-off-by: Haruki Imai <[email protected]>


          Add messages for Pooling and Convolution ops.

943b165

Signed-off-by: Haruki Imai <[email protected]>


          Merge branch 'main' into legality_check_msg

tungld reviewed

View reviewed changes

src/Accelerators/NNPA/Conversion/ONNXToZHigh/RewriteONNXForZHigh.cpp Outdated

+                          return !emitWarningMessageNNPAUnsupported(
+                              op.getOperation(), message); /* true */
+                        return false;

Collaborator

tungld May 20, 2024

This can be shorten in this way:

return (!sameBatchDims) || emitWarningMessageNNPAUnsupported(op.getOperation(), message);

Collaborator Author

imaihal May 21, 2024

Updated. Thanks!

imaihal added 2 commits

May 21, 2024 01:34


          Revert update for pooling.

bb090a3

Signed-off-by: Haruki Imai <[email protected]>


          Fix the check for pooling.

d3775e1

Signed-off-by: Haruki Imai <[email protected]>

Collaborator

tungld commented May 24, 2024

I updated to use --opt-report adding new enum value (NNPAUnsupportedOps). Thanks!

When it's ready for another round of review, please let me know. Thanks!

imaihal requested a review from tungld

May 24, 2024 07:43

Collaborator Author

imaihal commented May 24, 2024

Yes. ready for review.

tungld reviewed

View reviewed changes

Collaborator

tungld left a comment

Thanks for updating the patch. It's much better. Just some minor comments, but the most important thing is updating the document. And, feel free to merge the patch then.

In your example of roberta, I see this

==NNPA-UNSUPPORTEDOPS-REPORT== onnx.Softmax, /roberta/encoder/layer.0/attention/self/Softmax_176, The rank of input tensor (4) needs to be 2 or 3.

It seemed it was not applied in this model. Do you know what was the reason? Softmax with rank 4 is potentially rewritten to rank 2 by RewriteONNXForZHigh pass. Perhaps axis was not the last dimension in this model ...

src/Accelerators/Accelerator.hpp

@@ @@ -51,6 +51,10 @@ @@
               #define ACCEL_PROFILEIR_CL_ENUM(name) PROFILEIR_CL_ENUM_##name
+              #define ACCEL_OPTREPORT_ENUM(name) OPTREPORT_ENUM_##name
+              #define ACCEL_OPTREPORT_CL_ENUM(name) OPTREPORT_CL_ENUM_##name

Collaborator

tungld May 27, 2024

Could you update this document to include the new macro? Thanks!

Collaborator Author

imaihal Jun 5, 2024

Done. Thanks!

src/Accelerators/NNPA/Conversion/ONNXToZHigh/ONNXLegalityCheck.hpp Outdated

+              bool emitWarningMessageNNPAUnsupported(
+                  mlir::Operation *op, const std::string &message);
+              bool emitWarningMessageInCompatibility(mlir::Operation *op);

Collaborator

tungld May 27, 2024

Would you mind if we change these function names to make it consistent with functions for other --op-report options? We have onnxToKrnlSimdReport and onnxToKrnlParallelReport, so how about onnxToZHighUnsupportedReport? or feel free to choose another name.

Collaborator Author

imaihal Jun 5, 2024

Done. Thanks!

AlexandreEichenberger reviewed

View reviewed changes

Collaborator

AlexandreEichenberger left a comment

Could you add a , after the option identifier, namely transforming

==NNPA-UNSUPPORTEDOPS-REPORT== onnx.Mul, /roberta/embeddings/Mul, Element type is not F16 or F32.

to

==NNPA-UNSUPPORTEDOPS-REPORT==,  onnx.Mul, /roberta/embeddings/Mul, Element type is not F16 or F32.

so that is is like all of the other perf measurements; it let you do CSV imports more easily.

Great addition, thanks.

AlexandreEichenberger approved these changes

View reviewed changes

Collaborator

AlexandreEichenberger left a comment

Approving the PR, but please add the missing comma in the printout to be compatible with the other tools.


          Merge branch 'main' into legality_check_msg

30092eb

Collaborator Author

imaihal commented Jun 5, 2024 •

edited

Loading

In your example of roberta, I see this
==NNPA-UNSUPPORTEDOPS-REPORT== onnx.Softmax, /roberta/encoder/layer.0/attention/self/Softmax_176, The rank of input tensor (4) needs to be 2 or 3.
It seemed it was not applied in this model. Do you know what was the reason? Softmax with rank 4 is potentially rewritten to rank 2 by RewriteONNXForZHigh pass. Perhaps axis was not the last dimension in this model ...

I confirmed all softmax ops are lowered to zhigh.softmax in ZHighIR.

I found this message was generated in DevicePlacementPass which also calls legality check. In the DevicePlacementPass, is ONNXToZHighPass called after RewriteONNXForZHighPass? If not and ONNXToZHighPass checks original softmax op, I think this message is generated.

imaihal added 2 commits

June 5, 2024 07:36


          Add , in the messages.

eca8c83

Signed-off-by: Haruki Imai <[email protected]>


          Update docs, rename functions.

1ef8db1

Signed-off-by: Haruki Imai <[email protected]>

Collaborator Author

imaihal commented Jun 5, 2024

Could you add a , after the option identifier, namely transforming

==NNPA-UNSUPPORTEDOPS-REPORT== onnx.Mul, /roberta/embeddings/Mul, Element type is not F16 or F32.

to

==NNPA-UNSUPPORTEDOPS-REPORT==,  onnx.Mul, /roberta/embeddings/Mul, Element type is not F16 or F32.

Done. Thanks!

Collaborator

tungld commented Jun 5, 2024

I confirmed all softmax ops are lowered to zhigh.softmax in ZHighIR.

So the printed message was incorrect. I think you would fix that before merging.

is ONNXToZHighPass called after RewriteONNXForZHighPass?

Yes, it is in the code, but they are independent calls and used for analysis. The results of each call will be merged later. You can take a look at: https://github.com/onnx/onnx-mlir/blob/main/src/Accelerators/NNPA/Conversion/ONNXToZHigh/DevicePlacement.cpp#L196


          Add a config parameter to disable reporting in DevicePlacementPass.

fe3dd87

Signed-off-by: Haruki Imai <[email protected]>

Collaborator Author

imaihal commented Jun 5, 2024

Thanks for the comment!. I updated the PR to enable the reporting only in ONNXToZHighPass and disable it in DevicePlacementPass. This removes reporting about Softmax, as described in updated description of this PR.

Collaborator

tungld commented Jun 6, 2024

I updated the PR to enable the reporting only in ONNXToZHighPass and disable it in DevicePlacementPass. This removes reporting about Softmax, as described in updated description of this PR.

Thank you for the great work! Please merge it at your convenience time.

imaihal changed the title ~~[NNPA] Warning messages for legality check~~ [NNPA] An option to report why the operations are not run on NNPA (--opt-report=NNPAUnsupportedOps)

Collaborator Author

imaihal commented Jun 6, 2024

@jenkins-droid test this please

imaihal added the Ready to merge label

Collaborator Author

imaihal commented Jun 6, 2024

@jenkins-droid test this please


          empty commit

22764ce

Signed-off-by: Haruki Imai <[email protected]>

imaihal merged commit cf651e7 into onnx:main

7 checks passed

Collaborator

jenkins-droid commented Jun 6, 2024

Jenkins Linux s390x Build #14919 [push] [NNPA] Option to report ... started at 18:34

Collaborator

jenkins-droid commented Jun 6, 2024

Jenkins Linux amd64 Build #14914 [push] [NNPA] Option to report ... started at 17:34

Collaborator

jenkins-droid commented Jun 6, 2024

Jenkins Linux ppc64le Build #13944 [push] [NNPA] Option to report ... started at 18:45

Collaborator

jenkins-droid commented Jun 6, 2024

Jenkins Linux amd64 Build #14914 [push] [NNPA] Option to report ... passed after 1 hr 6 min

Collaborator

jenkins-droid commented Jun 7, 2024

Jenkins Linux s390x Build #14919 [push] [NNPA] Option to report ... passed after 1 hr 29 min

Collaborator

jenkins-droid commented Jun 7, 2024

Jenkins Linux ppc64le Build #13944 [push] [NNPA] Option to report ... passed after 1 hr 56 min

imaihal deleted the legality_check_msg branch

June 11, 2024 01:50

AlexandreEichenberger mentioned this pull request

yolov3 models fail to compile on NNPA #2850

Closed

imaihal mentioned this pull request

[NNPA] Add missing message for --opt-report=NNPAUnsupportedOps for MatMul op #2869

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels