From 54c4351c39680464891933bb9ce5ea2563eddb8f Mon Sep 17 00:00:00 2001
From: Geza Lore <gezalore@gmail.com>
Date: Fri, 28 Oct 2022 13:41:13 +0100
Subject: [PATCH] V3Const: Do not introduce redundant AstExtend

Fixes #3716
---
 src/V3Const.cpp                          | 21 +++++++++++++++------
 test_regress/t/t_const_sel_sel_extend.pl | 16 ++++++++++++++++
 test_regress/t/t_const_sel_sel_extend.v  | 22 ++++++++++++++++++++++
 3 files changed, 53 insertions(+), 6 deletions(-)
 create mode 100755 test_regress/t/t_const_sel_sel_extend.pl
 create mode 100644 test_regress/t/t_const_sel_sel_extend.v

diff --git a/src/V3Const.cpp b/src/V3Const.cpp
index 158561f07..9d1b7f7f8 100644
--- a/src/V3Const.cpp
+++ b/src/V3Const.cpp
@@ -2411,12 +2411,21 @@ private:
             VL_DO_DANGLING(lsb1p->deleteTree(), lsb1p);
             VL_DO_DANGLING(lsb2p->deleteTree(), lsb2p);
         } else {
-            // Width is important, we need the width of the fromp's
-            // expression, not the potentially smaller lsb1p's width
-            newlsbp
-                = new AstAdd(lsb1p->fileline(), lsb2p, new AstExtend(lsb1p->fileline(), lsb1p));
-            newlsbp->dtypeFrom(lsb2p);  // Unsigned
-            VN_AS(newlsbp, Add)->rhsp()->dtypeFrom(lsb2p);
+            // Width is important, we need the width of the fromp's expression, not the
+            // potentially smaller lsb1p's width, but don't insert a redundant AstExtend.
+            // Note that due to some sloppiness in earlier passes, lsb1p might actually be wider,
+            // so extend to the wider type.
+            AstNode* const widep = lsb1p->width() > lsb2p->width() ? lsb1p : lsb2p;
+            AstNode* const lhsp = widep->width() > lsb2p->width()
+                                      ? new AstExtend{lsb2p->fileline(), lsb2p}
+                                      : lsb2p;
+            AstNode* const rhsp = widep->width() > lsb1p->width()
+                                      ? new AstExtend{lsb1p->fileline(), lsb1p}
+                                      : lsb1p;
+            lhsp->dtypeFrom(widep);
+            rhsp->dtypeFrom(widep);
+            newlsbp = new AstAdd{lsb1p->fileline(), lhsp, rhsp};
+            newlsbp->dtypeFrom(widep);
         }
         AstSel* const newp = new AstSel(nodep->fileline(), fromp, newlsbp, widthp);
         nodep->replaceWith(newp);
diff --git a/test_regress/t/t_const_sel_sel_extend.pl b/test_regress/t/t_const_sel_sel_extend.pl
new file mode 100755
index 000000000..84ae125be
--- /dev/null
+++ b/test_regress/t/t_const_sel_sel_extend.pl
@@ -0,0 +1,16 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2022 by Geza Lore. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(vlt => 1);
+
+compile();
+
+ok(1);
+1;
diff --git a/test_regress/t/t_const_sel_sel_extend.v b/test_regress/t/t_const_sel_sel_extend.v
new file mode 100644
index 000000000..c695de4f2
--- /dev/null
+++ b/test_regress/t/t_const_sel_sel_extend.v
@@ -0,0 +1,22 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2022 by Geza Lore.
+// SPDX-License-Identifier: CC0-1.0
+
+module t(
+  output wire res
+);
+
+   function automatic logic foo(logic bar);
+      foo = '0;
+   endfunction
+
+   logic       a, b;
+   logic [0:0][1:0] array;
+
+   assign b = 0;
+   assign a = foo(b);
+   assign res = array[a][a];
+
+endmodule