You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tvm.apache.org by Krzysztof Parzyszek via Apache TVM Discuss <no...@discuss.tvm.ai> on 2022/02/24 23:57:38 UTC
[Apache TVM Discuss] [Development] Problem with FuseOps (and embedded constants in TIR)
Long story short, after https://github.com/apache/tvm/pull/8509 we see a lot of floating point models crash during compilation for Hexagon. On Hexagon (downstream) we use the mixed-precision pass to convert float32 data to float16. Float16 constants are not supported by constants in TIR and compilation aborts.
Longer story...
The relay pass `FuseOps` is the one that can extract constants out of expressions, and replace them with parameters. Constants that are not parameters cannot have type float16, because that type is not supported by current TIR code (for embedded constants). If this extraction doesn't happen, compilation will abort if a float16 constant is found in TIR.
After commit b5f1dabce4 (PR8509), `FuseOps` will no longer extract constants if the build target has `link_params` set to true. This can be, at least in theory, overridden by pass context flag `relay.FuseOps.link_params`, but there is an issue.
The problem is as follows:
* The `FoldConstant` pass runs, and it creates a fresh `PassContext`, without any config flags in it. It also uses the "cpu" target instead of the actual one for `CompilationConfig`.
* During execution, `FoldConstant` will invoke relay interpreter's function `Eval`, which initiates a series of relay passes, including `FuseOps`.
* `FuseOps` gets `link_params` value from `IRModule`'s attribute `Executor`, which is still consistent with the actual target. If the original target has `link_params = 1`, it will take effect regardless of any flags added to PassContext at the `relay.Build` time.
It seems like `FuseOps` should be getting settings consistent with the CPU target that `FoldConstants` created, instead of using `Executor` from `IRModule`. This difference may have further consequences if passes executed during `FoldConstants` consult the executor for more information.
Any thoughts?
---
[Visit Topic](https://discuss.tvm.apache.org/t/problem-with-fuseops-and-embedded-constants-in-tir/12165/1) to respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click here](https://discuss.tvm.apache.org/email/unsubscribe/0b6ce7e6937a48ba45f2697ea4cb66f7a2310582dd0902276e7f45fc523f64be).
[Apache TVM Discuss] [Development] Problem with FuseOps (and embedded constants in TIR)
Posted by Krzysztof Parzyszek via Apache TVM Discuss <no...@discuss.tvm.ai>.
Sure, I replied in the [PR](https://github.com/apache/tvm/pull/8509).
---
[Visit Topic](https://discuss.tvm.apache.org/t/problem-with-fuseops-and-embedded-constants-in-tir/12165/8) to respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click here](https://discuss.tvm.apache.org/email/unsubscribe/9318353dcf1757561f21aa40d10223d2a09503e2e778366e875e374e037cd8a6).
[Apache TVM Discuss] [Development] Problem with FuseOps (and embedded constants in TIR)
Posted by Manupa Karunaratne via Apache TVM Discuss <no...@discuss.tvm.ai>.
@kparzysz .
As mentioned in the PR, the above reference is about scalar constants, that is not subject to link-params. (Correct me if I am wrong -- @dmitriy-arm ).
#8509 is about non-scalar constants.
One option is to hexagon backend needs to be adjusted to handle AllocateConst nodes, instead of LinkedParams node.
Lets continue the discussion on the PR (#8509) to avoid duplicate conversation.
Do you agree @kparzysz ?
---
[Visit Topic](https://discuss.tvm.apache.org/t/problem-with-fuseops-and-embedded-constants-in-tir/12165/7) to respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click here](https://discuss.tvm.apache.org/email/unsubscribe/26d1cc64e80ad42468fb03b4fa57fb61451a3cd971cab7966ffa5bb3cc7948d9).
[Apache TVM Discuss] [Development] Problem with FuseOps (and embedded constants in TIR)
Posted by Krzysztof Parzyszek via Apache TVM Discuss <no...@discuss.tvm.ai>.
We were hitting this assertion:
https://github.com/apache/tvm/blob/main/src/relay/backend/te_compiler_cache.cc#L242
We do have a unit testcase that checks the LLVM IR to see if the _linked_param function was generated, but I'm not sure if it runs in CI right now, since it requires Hexagon target to be enabled. This is actually work in progress, so these tests should start running soon.
---
[Visit Topic](https://discuss.tvm.apache.org/t/problem-with-fuseops-and-embedded-constants-in-tir/12165/6) to respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click here](https://discuss.tvm.apache.org/email/unsubscribe/7550c8a313ce65710ba90814915552a78961d693c51cd6c6d559551cdd239150).
[Apache TVM Discuss] [Development] Problem with FuseOps (and embedded constants in TIR)
Posted by Manupa Karunaratne via Apache TVM Discuss <no...@discuss.tvm.ai>.
Hi @kparzysz ,
Sorry to hear that there was downstream failure because of #8509.
[quote="kparzysz, post:1, topic:12165"]
Float16 constants are not supported by constants in TIR and compilation aborts.
[/quote]
[quote="kparzysz, post:1, topic:12165"]
Constants that are not parameters cannot have type float16, because that type is not supported by current TIR code (for embedded constants). If this extraction doesn’t happen, compilation will abort if a float16 constant is found in TIR.
[/quote]
[quote="masahi, post:3, topic:12165"]
I wonder why TIR constants doesn’t support fp16? Because of the need for c-codegen?
[/quote]
I am also wondering how this is true, because of the following (note that we just used what was supported in TVM via LinkedParam node) :
https://github.com/apache/tvm/blob/dcbdeddba290caee4f604e678a7e315aea038881/src/target/llvm/codegen_params.cc#L137-L157
https://github.com/apache/tvm/blob/dcbdeddba290caee4f604e678a7e315aea038881/src/target/source/codegen_params.cc#L214-L229
The above are called respectively in the following locations :
https://github.com/apache/tvm/blob/dcbdeddba290caee4f604e678a7e315aea038881/src/target/llvm/codegen_llvm.cc#L1485
https://github.com/apache/tvm/blob/dcbdeddba290caee4f604e678a7e315aea038881/src/target/source/codegen_c.cc#L673
[quote="kparzysz, post:1, topic:12165"]
This can be, at least in theory, overridden by pass context flag `relay.FuseOps.link_params`, but there is an issue.
[/quote]
[quote="kparzysz, post:1, topic:12165"]
It seems like `FuseOps` should be getting settings consistent with the CPU target that `FoldConstants` created, instead of using `Executor` from `IRModule`. This difference may have further consequences if passes executed during `FoldConstants` consult the executor for more information.
[/quote]
I think we need to move the link-params to be a property of the TIR backend (i.e. target).
We need to fix this if it is not happenning. @kparzysz would you be able to file an issue ?
attn : @dmitriy-arm @Mousius
However, the above seems to be the workaround of the issue, the real issue I suspect is hexagon backend expect LinkedParams if --link-params is used, however, there is not a unit test (a simple codegen one) to assert this -- that should have been broken by #8509.
---
[Visit Topic](https://discuss.tvm.apache.org/t/problem-with-fuseops-and-embedded-constants-in-tir/12165/5) to respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click here](https://discuss.tvm.apache.org/email/unsubscribe/9a9ea5258496cff4bd8d18f61d8667397cdf37126ff595badae99321beb4cee2).
[Apache TVM Discuss] [Development] Problem with FuseOps (and embedded constants in TIR)
Posted by Siyuan Feng via Apache TVM Discuss <no...@discuss.tvm.ai>.
I'm not sure. But I guess it is because C++ doesn't have a native fp16 type support?
---
[Visit Topic](https://discuss.tvm.apache.org/t/problem-with-fuseops-and-embedded-constants-in-tir/12165/4) to respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click here](https://discuss.tvm.apache.org/email/unsubscribe/3da513b72c48b74604e983a355e6ca1520e54f0596eed145aac7ca6b9f2e136d).
[Apache TVM Discuss] [Development] Problem with FuseOps (and embedded constants in TIR)
Posted by masahi via Apache TVM Discuss <no...@discuss.tvm.ai>.
I wonder why TIR constants doesn't support fp16? Because of the need for c-codegen? @manupa-arm
---
[Visit Topic](https://discuss.tvm.apache.org/t/problem-with-fuseops-and-embedded-constants-in-tir/12165/3) to respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click here](https://discuss.tvm.apache.org/email/unsubscribe/d68e59e59a3d2082a6277534f5c3a03950c7ae78e5609e179624f69d08dbfbc4).
[Apache TVM Discuss] [Development] Problem with FuseOps (and embedded constants in TIR)
Posted by Mark Shields via Apache TVM Discuss <no...@discuss.tvm.ai>.
Agree with your last sentence -- FoldConstants should be CPU only and not carry forward any target-specific flags. (Ideally do all that more directly instead of piggy-backing on the interpreter, but that's a bigger issue.)
---
[Visit Topic](https://discuss.tvm.apache.org/t/problem-with-fuseops-and-embedded-constants-in-tir/12165/2) to respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click here](https://discuss.tvm.apache.org/email/unsubscribe/c9e0c7fa4a03237cfa320f41da3a598dc290705394bbdbe8b52ad253308b3298).