areusch commented on a change in pull request #22:
URL: https://github.com/apache/tvm-rfcs/pull/22#discussion_r692378556



##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.

Review comment:
       nit: say "non-scalar constants are represented in Relay" 

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.

Review comment:
       flip the second sentence around: "Therefore, when a compiler pass is 
operating on TIR, it needs additional input to determine whether a `tir::Var` 
is runtime-supplied data or a compile-time constant (and what that constant 
data is). Should that pass want to transform the constant data, the additional 
input also becomes an output. In order to provide this data, the compiler has 
to maintain a side-channel map, complicating the Pass interface."

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.
+
+## Memory Planning
+
+The TIR program has the ability to express both inter and intra operator 
memory requirement, post-scheduling as explained further by [Unified Static 
Memory Planning RFC](https://github.com/apache/tvm-rfcs/pull/9). It would be 
better if the constants could be embedded to the TIR PrimFunc. Moreover, this 
allows various [target-dependent 
lowerings](https://github.com/apache/tvm-rfcs/pull/10), to produce TIR 
PrimFuncs with constants in it.
+
+## Winograd Constants
+
+The Winograd transformation (used for fast GEMMs) involves multiplication by a 
hard-coded constant tensor. This is currently accomplished in TE using a 
complicated TE compute expression with many nested selects. Being able to 
directly express a constant tensor here would significantly simplify this code.
+
+
+# 3. Guide-level explanation
+
+This is not particularly a user-facing feature and this will allow constants 
to be 'linked' to TIR. Initially, we are planning to use this with gated on 
'-link-params' argument for relay.build and TVMC.
+
+# 4. Reference-level explanation

Review comment:
       does the example from the other RFC still hold? if so, worth copying 
here?

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.
+
+## Memory Planning
+
+The TIR program has the ability to express both inter and intra operator 
memory requirement, post-scheduling as explained further by [Unified Static 
Memory Planning RFC](https://github.com/apache/tvm-rfcs/pull/9). It would be 
better if the constants could be embedded to the TIR PrimFunc. Moreover, this 
allows various [target-dependent 
lowerings](https://github.com/apache/tvm-rfcs/pull/10), to produce TIR 
PrimFuncs with constants in it.
+
+## Winograd Constants
+
+The Winograd transformation (used for fast GEMMs) involves multiplication by a 
hard-coded constant tensor. This is currently accomplished in TE using a 
complicated TE compute expression with many nested selects. Being able to 
directly express a constant tensor here would significantly simplify this code.
+
+
+# 3. Guide-level explanation
+
+This is not particularly a user-facing feature and this will allow constants 
to be 'linked' to TIR. Initially, we are planning to use this with gated on 
'-link-params' argument for relay.build and TVMC.
+
+# 4. Reference-level explanation
+
+The proposal is quite simple and it could be explained as follows :
+
+```
+@tvm.script.tir
+def myfunc():   
+   param = tir.allocate_const([1, 1, 1, 1, 1, 1, 1, 1, 1, 1], "int32", [10])
+```
+
+This follows closely the semantics of tir.allocate and the difference being it 
represent a buffer filled with constants.
+
+There are mainly two ways of constants being created in the lowering :
+
+A1. Linking the params of the model (relay.Constants)
+
+A2. Creation of constants in the lowering.

Review comment:
       though i agree lower is a function, at this level could you clarify 
("when scheduling relay into TIR"). Could you also indicate that this is 
different from A1 in that the constants are created by the scheduling system 
and therefore can be directly embedded into the operator function?

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.
+
+## Memory Planning
+
+The TIR program has the ability to express both inter and intra operator 
memory requirement, post-scheduling as explained further by [Unified Static 
Memory Planning RFC](https://github.com/apache/tvm-rfcs/pull/9). It would be 
better if the constants could be embedded to the TIR PrimFunc. Moreover, this 
allows various [target-dependent 
lowerings](https://github.com/apache/tvm-rfcs/pull/10), to produce TIR 
PrimFuncs with constants in it.
+
+## Winograd Constants
+
+The Winograd transformation (used for fast GEMMs) involves multiplication by a 
hard-coded constant tensor. This is currently accomplished in TE using a 
complicated TE compute expression with many nested selects. Being able to 
directly express a constant tensor here would significantly simplify this code.

Review comment:
       do you mind including a link to code for those not acquainted?

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.

Review comment:
       nit: maybe "see ARM(R) Ethos(TM)-U NPU RFC"

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.
+
+## Memory Planning
+
+The TIR program has the ability to express both inter and intra operator 
memory requirement, post-scheduling as explained further by [Unified Static 
Memory Planning RFC](https://github.com/apache/tvm-rfcs/pull/9). It would be 
better if the constants could be embedded to the TIR PrimFunc. Moreover, this 
allows various [target-dependent 
lowerings](https://github.com/apache/tvm-rfcs/pull/10), to produce TIR 
PrimFuncs with constants in it.

Review comment:
       > It would be better
   
   I believe you, but state why. Also state the advantage of producing TIR 
PrimFuncs in target-depdendent lowerings.

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.
+
+## Memory Planning
+
+The TIR program has the ability to express both inter and intra operator 
memory requirement, post-scheduling as explained further by [Unified Static 
Memory Planning RFC](https://github.com/apache/tvm-rfcs/pull/9). It would be 
better if the constants could be embedded to the TIR PrimFunc. Moreover, this 
allows various [target-dependent 
lowerings](https://github.com/apache/tvm-rfcs/pull/10), to produce TIR 
PrimFuncs with constants in it.
+
+## Winograd Constants
+
+The Winograd transformation (used for fast GEMMs) involves multiplication by a 
hard-coded constant tensor. This is currently accomplished in TE using a 
complicated TE compute expression with many nested selects. Being able to 
directly express a constant tensor here would significantly simplify this code.
+
+
+# 3. Guide-level explanation
+
+This is not particularly a user-facing feature and this will allow constants 
to be 'linked' to TIR. Initially, we are planning to use this with gated on 
'-link-params' argument for relay.build and TVMC.
+
+# 4. Reference-level explanation
+
+The proposal is quite simple and it could be explained as follows :
+
+```
+@tvm.script.tir
+def myfunc():   
+   param = tir.allocate_const([1, 1, 1, 1, 1, 1, 1, 1, 1, 1], "int32", [10])
+```
+
+This follows closely the semantics of tir.allocate and the difference being it 
represent a buffer filled with constants.
+
+There are mainly two ways of constants being created in the lowering :
+
+A1. Linking the params of the model (relay.Constants)
+
+A2. Creation of constants in the lowering.
+
+For A1, this should only be done if the target support codegeneration of the 
constant data as part of the operators.

Review comment:
       Since it's not possible to do this for graph-level constants outside AOT 
flow, specify the future plan

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.
+
+## Memory Planning
+
+The TIR program has the ability to express both inter and intra operator 
memory requirement, post-scheduling as explained further by [Unified Static 
Memory Planning RFC](https://github.com/apache/tvm-rfcs/pull/9). It would be 
better if the constants could be embedded to the TIR PrimFunc. Moreover, this 
allows various [target-dependent 
lowerings](https://github.com/apache/tvm-rfcs/pull/10), to produce TIR 
PrimFuncs with constants in it.
+
+## Winograd Constants
+
+The Winograd transformation (used for fast GEMMs) involves multiplication by a 
hard-coded constant tensor. This is currently accomplished in TE using a 
complicated TE compute expression with many nested selects. Being able to 
directly express a constant tensor here would significantly simplify this code.
+
+
+# 3. Guide-level explanation
+
+This is not particularly a user-facing feature and this will allow constants 
to be 'linked' to TIR. Initially, we are planning to use this with gated on 
'-link-params' argument for relay.build and TVMC.
+
+# 4. Reference-level explanation
+
+The proposal is quite simple and it could be explained as follows :
+
+```
+@tvm.script.tir
+def myfunc():   
+   param = tir.allocate_const([1, 1, 1, 1, 1, 1, 1, 1, 1, 1], "int32", [10])
+```
+
+This follows closely the semantics of tir.allocate and the difference being it 
represent a buffer filled with constants.
+
+There are mainly two ways of constants being created in the lowering :
+
+A1. Linking the params of the model (relay.Constants)

Review comment:
       can you briefly state how exactly these are produced (e.g. bound relay 
Var are translated to tir.Constant <specify the function where these are 
created>). 

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.
+
+## Memory Planning
+
+The TIR program has the ability to express both inter and intra operator 
memory requirement, post-scheduling as explained further by [Unified Static 
Memory Planning RFC](https://github.com/apache/tvm-rfcs/pull/9). It would be 
better if the constants could be embedded to the TIR PrimFunc. Moreover, this 
allows various [target-dependent 
lowerings](https://github.com/apache/tvm-rfcs/pull/10), to produce TIR 
PrimFuncs with constants in it.
+
+## Winograd Constants
+
+The Winograd transformation (used for fast GEMMs) involves multiplication by a 
hard-coded constant tensor. This is currently accomplished in TE using a 
complicated TE compute expression with many nested selects. Being able to 
directly express a constant tensor here would significantly simplify this code.
+
+
+# 3. Guide-level explanation
+
+This is not particularly a user-facing feature and this will allow constants 
to be 'linked' to TIR. Initially, we are planning to use this with gated on 
'-link-params' argument for relay.build and TVMC.
+
+# 4. Reference-level explanation
+
+The proposal is quite simple and it could be explained as follows :
+
+```
+@tvm.script.tir
+def myfunc():   
+   param = tir.allocate_const([1, 1, 1, 1, 1, 1, 1, 1, 1, 1], "int32", [10])
+```
+
+This follows closely the semantics of tir.allocate and the difference being it 
represent a buffer filled with constants.
+
+There are mainly two ways of constants being created in the lowering :
+
+A1. Linking the params of the model (relay.Constants)
+
+A2. Creation of constants in the lowering.
+
+For A1, this should only be done if the target support codegeneration of the 
constant data as part of the operators.
+
+For A2, the lowering for targets that support constant as part of the 
operators, there can be new (differently sized) constants could be created due 
to optimizations such as weight compression as required by the target.

Review comment:
       can you explain how the compiler knows which targets support this?

##########
File path: rfcs/0022-tir-non-scalar-constants.md
##########
@@ -0,0 +1,107 @@
+
+- Feature Name: tir_non_scalar_constants
+- Start Date: 2021-06-01
+- RFC PR: https://github.com/apache/tvm-rfcs/pull/22
+- GitHub Issue: TBD
+
+# 1. Summary
+
+This RFC proposes how non-scalar constants could be represented in TIR and 
used by passes in the lowering process.
+
+# 2. Motivation 
+
+Currently, the non-scalar constants could be represented in Relay 
(relay.Constant) to be used by relay passes but not in TIR. Therefore, when 
performing lowering using TIR passes, we have to maintain a side-channel of 
tir::Var to constant non-scalar data mapping to perform transformations that 
could use the knowledge where some of the data are constants.
+
+Few example scenarios as further motivation :
+
+## Weight compression
+
+When lowering for accelerators (E.g. : [Arm(R) Ethos(TM)-U 
NPU](https://github.com/apache/tvm-rfcs/pull/11)), certain operations will need 
to get tiled to co-optimize performance and memory utilization. Such tiling 
patterns create slices of weights that need compressing that will end up with 
varying sizes. Therefore, the knowledge of some tir::Vars refer to constants 
are critical in the level of TIR to perform this.
+
+## Memory Planning
+
+The TIR program has the ability to express both inter and intra operator 
memory requirement, post-scheduling as explained further by [Unified Static 
Memory Planning RFC](https://github.com/apache/tvm-rfcs/pull/9). It would be 
better if the constants could be embedded to the TIR PrimFunc. Moreover, this 
allows various [target-dependent 
lowerings](https://github.com/apache/tvm-rfcs/pull/10), to produce TIR 
PrimFuncs with constants in it.
+
+## Winograd Constants
+
+The Winograd transformation (used for fast GEMMs) involves multiplication by a 
hard-coded constant tensor. This is currently accomplished in TE using a 
complicated TE compute expression with many nested selects. Being able to 
directly express a constant tensor here would significantly simplify this code.
+
+
+# 3. Guide-level explanation
+
+This is not particularly a user-facing feature and this will allow constants 
to be 'linked' to TIR. Initially, we are planning to use this with gated on 
'-link-params' argument for relay.build and TVMC.

Review comment:
       say "Intially, tir.Constant will only be created during scheduling when 
`-link-params` is included in the Target (e.g. to `relay.build` and to TVMC)"




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to