From 94d1bee4c23bd3d8f99dae3ca431ffaa5dc1410d Mon Sep 17 00:00:00 2001
From: Jack Koenig
Date: Tue, 16 Mar 2021 10:26:38 -0700
Subject: Fix issue where inlined cvt could cause crash (#2124)

Due to inlining of Boolean expressions, the following circuit is handled
directly by the VerilogEmitter:

input a: UInt<4>
input b: SInt<1>
output o: UInt<5>
o <= dshl(a, asUInt(cvt(b)))

Priot to this change, this could crash due to mishandling of cvt in the
logic to inject parentheses based on Verilog precedence rules.

This is a corner case, but similar bugs would drop up if we open up the
VerilogEmitter to more expression inlining.---
 src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

(limited to 'src/main')

diff --git a/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala b/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala
index c7143f5f..33c6b9a8 100644
--- a/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala
+++ b/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala
@@ -57,6 +57,13 @@ object VerilogEmitter {
   private def precedenceGt(op1: PrimOp, op2: PrimOp): Boolean = {
     precedenceMap(op1) < precedenceMap(op2)
   }
+
+  /** Identifies PrimOps that never need parentheses
+    *
+    * These PrimOps emit either {..., a0, ...} or a0 so they never need parentheses
+    */
+  private val neverParens: PrimOp => Boolean =
+    Set(Shl, Cat, Cvt, AsUInt, AsSInt, AsClock, AsAsyncReset, Pad)
 }
 
 class VerilogEmitter extends SeqTransform with Emitter {
@@ -229,8 +236,7 @@ class VerilogEmitter extends SeqTransform with Emitter {
   // to ensure Verilog operations are signed.
   def op_stream(doprim: DoPrim): Seq[Any] = {
     def parenthesize(e: Expression, isFirst: Boolean): Any = doprim.op match {
-      // these PrimOps emit either {..., a0, ...} or a0 so they never need parentheses
-      case Shl | Cat | Cvt | AsUInt | AsSInt | AsClock | AsAsyncReset => e
+      case op if neverParens(op) => e
       case _ =>
         e match {
           case e: DoPrim =>
@@ -247,7 +253,8 @@ class VerilogEmitter extends SeqTransform with Emitter {
                */
               case other =>
                 val noParens =
-                  precedenceGt(e.op, doprim.op) ||
+                  neverParens(e.op) ||
+                    precedenceGt(e.op, doprim.op) ||
                     (isFirst && precedenceEq(e.op, doprim.op) && !isUnaryOp(e.op))
                 if (noParens) other else Seq("(", other, ")")
             }
-- 
cgit v1.2.3