chisel-tutorial icon indicating copy to clipboard operation
chisel-tutorial copied to clipboard

question about Firrtl(IR) optimization

Open YingkunZhou opened this issue 6 years ago • 11 comments
trafficstars

// See LICENSE.txt for license details.
package solutions

import chisel3._

// Problem:
//
// 'out' should be the sum of 'in0' and 'in1'
// Adder width should be parametrized
//
class Adder(val w: Int) extends Module {
  val io = IO(new Bundle {
    val in0 = Input(UInt(w.W))
    val in1 = Input(UInt(w.W))
    val out = Output(UInt(w.W))
  })
  io.out := io.in0 + io.in1
}

above is the scala code for Adder in $TUT_DIR/src/main/scala/problems/ when I run ./run-solution.sh Adder --backend-name verilator I got the corresponding Verilog code module Adder( input clock, input reset, input [7:0] io_in0, input [7:0] io_in1, output [7:0] io_out ); wire [8:0] _T_11; // @[Adder.scala 17:20] assign _T_11 = io_in0 + io_in1; // @[Adder.scala 17:20] assign io_out = io_in0 + io_in1; // @[Adder.scala 17:10] endmodule

Here assign _T_11 = io_in0 + io_in1; // @[Adder.scala 17:20] is a line of deadcode, which in my eyes wastes a build-in adder thus should be eliminated. And it is easy to fulfill using dead code elimination for IR. So is it a bug for current chisel3 implementation?

YingkunZhou avatar Dec 22 '18 01:12 YingkunZhou

While I'm not sure why FIRRTL generated _T_11, synthesis tools will almost always prune those kinds of lines so it will not consume any physical resources.

edwardcwang avatar Dec 22 '18 05:12 edwardcwang

But you cannot expect that synthesis tools will do these things.

YingkunZhou avatar Dec 22 '18 07:12 YingkunZhou

From our experience it usually does. Feel free to ask some others or try it yourself.

edwardcwang avatar Dec 22 '18 07:12 edwardcwang

@YingkunZhou it is definitely fair to say that _T_11 shouldn't be there, regardless of how it impacts final QoR. Is there firrtl output you can also post? It should be in the same directory as the verilog and have a ".fir" file extension. I think this is a backend problem more than a frontend problem, but having firrtl would be helpful in verifying that. Perhaps something is being const-prop'd and then DCE is not being run afterwards.

grebe avatar Dec 22 '18 14:12 grebe

;buildInfoPackage: chisel3, version: 3.1.5, scalaVersion: 2.11.12, sbtVersion: 1.1.1, builtAtString: 2018-12-14 23:45:23.572, builtAtMillis: 1544831123572 circuit Adder : module Adder : input clock : Clock input reset : UInt<1> output io : {flip in0 : UInt<8>, flip in1 : UInt<8>, out : UInt<8>}

node _T_11 = add(io.in0, io.in1) @[Adder.scala 17:20]
node _T_12 = tail(_T_11, 1) @[Adder.scala 17:20]
io.out <= _T_12 @[Adder.scala 17:10]

YingkunZhou avatar Dec 22 '18 21:12 YingkunZhou

not familiar with firrtl, though

YingkunZhou avatar Dec 22 '18 21:12 YingkunZhou

My output is

module Adder( // @[:[email protected]]
  input        clock, // @[:[email protected]]
  input        reset, // @[:[email protected]]
  input  [7:0] io_in0, // @[:[email protected]]
  input  [7:0] io_in1, // @[:[email protected]]
  output [7:0] io_out // @[:[email protected]]
);
  assign io_out = io_in0 + io_in1; // @[Adder.scala 17:10:[email protected]]
endmodule

Perhaps the tutorial is using an older version of firrtl?

grebe avatar Dec 23 '18 00:12 grebe

so you use master branch? I forget when and how I installed firrtl

YingkunZhou avatar Dec 23 '18 00:12 YingkunZhou

Did the default behavior of + change to +& instead of +%? By default firrtl width inference adds an extra bit to the output of add and it has to trim it back down to the original width (that you specified for the output). Was it ever smart enough to get rid of the extra T? If you left the output width as unknown, it wouldn’t generate the extra T, I think, and would give you an output with width w + 1. I also don’t really like relying on tools to be able to do optimization’s that firrtl should be able to take care of either.

On Saturday, December 22, 2018, YingkunZhou [email protected] wrote:

so you use master branch? I forget when and how I installed firrtl

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ucb-bar/chisel-tutorial/issues/142#issuecomment-449606736, or mute the thread https://github.com/notifications/unsubscribe-auth/AGTTFpk6nLrMrV9sseoCIBalhQfrwNiCks5u7tL1gaJpZM4ZfQ9k .

shunshou avatar Dec 23 '18 01:12 shunshou

Oops didn’t see Paul’s last reply.

On Saturday, December 22, 2018, Angie [email protected] wrote:

Did the default behavior of + change to +& instead of +%? By default firrtl width inference adds an extra bit to the output of add and it has to trim it back down to the original width (that you specified for the output). Was it ever smart enough to get rid of the extra T? If you left the output width as unknown, it wouldn’t generate the extra T, I think, and would give you an output with width w + 1. I also don’t really like relying on tools to be able to do optimization’s that firrtl should be able to take care of either.

On Saturday, December 22, 2018, YingkunZhou [email protected] wrote:

so you use master branch? I forget when and how I installed firrtl

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ucb-bar/chisel-tutorial/issues/142#issuecomment-449606736, or mute the thread https://github.com/notifications/unsubscribe-auth/AGTTFpk6nLrMrV9sseoCIBalhQfrwNiCks5u7tL1gaJpZM4ZfQ9k .

shunshou avatar Dec 23 '18 01:12 shunshou

You may not have installed firrtl- it's included as a dependency of chisel (see here). Perhaps forcing the latest 3.1.5 for chisel will fix things? The release is a little over a week old.

grebe avatar Dec 23 '18 15:12 grebe