question about Firrtl(IR) optimization

[YingkunZhou]

// 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?

[edwardcwang]

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.

[YingkunZhou]

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

[edwardcwang]

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

[grebe]

@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.

[YingkunZhou]

;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]

not familiar with firrtl, though

[grebe]

My output is

module Adder( // @[:test.fir@2.2]
  input        clock, // @[:test.fir@3.4]
  input        reset, // @[:test.fir@4.4]
  input  [7:0] io_in0, // @[:test.fir@5.4]
  input  [7:0] io_in1, // @[:test.fir@5.4]
  output [7:0] io_out // @[:test.fir@5.4]
);
  assign io_out = io_in0 + io_in1; // @[Adder.scala 17:10:test.fir@9.4]
endmodule

Perhaps the tutorial is using an older version of firrtl?

[YingkunZhou]

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

[shunshou]

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 ***@***.***> 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 <#142 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGTTFpk6nLrMrV9sseoCIBalhQfrwNiCks5u7tL1gaJpZM4ZfQ9k> .

[shunshou]

Oops didn’t see Paul’s last reply.

…

On Saturday, December 22, 2018, Angie ***@***.***> 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 ***@***.***> 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 > <#142 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AGTTFpk6nLrMrV9sseoCIBalhQfrwNiCks5u7tL1gaJpZM4ZfQ9k> > . >

[grebe]

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.