From 095b1ccb6772259082301c672afa2273c60b6688 Mon Sep 17 00:00:00 2001 From: Yutetsu TAKATSUKASA Date: Wed, 17 Jul 2024 18:54:58 +0900 Subject: [PATCH] Fix incorrect result of width mismatch (#5186) (#5189) --- src/V3Const.cpp | 3 ++- test_regress/t/t_const_opt.pl | 2 +- test_regress/t/t_const_opt.v | 25 ++++++++++++++++++++++++- test_regress/t/t_const_opt_dfg.pl | 2 +- test_regress/t/t_debug_width.out | 4 ++-- 5 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/V3Const.cpp b/src/V3Const.cpp index e18fd2f02..403148bf8 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -853,7 +853,8 @@ public: // of the masking node e.g. by the "AND with all ones" rule. If the result width happens // to be 1, we still need to ensure the AstAnd is not dropped, so use a wider mask in this // special case. - const int maskWidth = resultWidth == 1 ? VL_IDATASIZE : resultWidth; + const int maskWidth + = std::max(resultp->width(), resultWidth == 1 ? VL_IDATASIZE : resultWidth); // Apply final polarity flip if (needsFlip) { diff --git a/test_regress/t/t_const_opt.pl b/test_regress/t/t_const_opt.pl index 9ce435f7b..f97a44629 100755 --- a/test_regress/t/t_const_opt.pl +++ b/test_regress/t/t_const_opt.pl @@ -20,7 +20,7 @@ execute( ); if ($Self->{vlt}) { - file_grep($Self->{stats}, qr/Optimizations, Const bit op reduction\s+(\d+)/i, 42); + file_grep($Self->{stats}, qr/Optimizations, Const bit op reduction\s+(\d+)/i, 45); } ok(1); 1; diff --git a/test_regress/t/t_const_opt.v b/test_regress/t/t_const_opt.v index b60aa75fa..5abf755f1 100644 --- a/test_regress/t/t_const_opt.v +++ b/test_regress/t/t_const_opt.v @@ -97,10 +97,11 @@ module Test(/*AUTOARG*/ logic bug4837_out; logic bug4857_out; logic bug4864_out; + logic bug5186_out; output logic o; - logic [18:0] tmp; + logic [19:0] tmp; assign o = ^tmp; always_ff @(posedge clk) begin @@ -135,6 +136,7 @@ module Test(/*AUTOARG*/ tmp[16]<= bug4837_out; tmp[17]<= bug4857_out; tmp[18]<= bug4864_out; + tmp[19]<= bug5186_out; end bug3182 i_bug3182(.in(d[4:0]), .out(bug3182_out)); @@ -150,6 +152,7 @@ module Test(/*AUTOARG*/ bug4837 i_bug4837(.clk(clk), .in(d), .out(bug4837_out)); bug4857 i_bug4857(.clk(clk), .in(d), .out(bug4857_out)); bug4864 i_bug4864(.clk(clk), .in(d), .out(bug4864_out)); + bug5186 i_bug5186(.clk(clk), .in(d), .out(bug5186_out)); endmodule @@ -543,3 +546,23 @@ module bug4864(input wire clk, input wire [31:0] in, output wire out); assign out = sig_e; endmodule + + +// See issue #5168 +// BitOpTree removes d[38] ^ d[38] correctly, but adds a cleaning AND as +// (d[31] & 32'b1) even though d is 64 bit width. +// matchMaskedShift() thinks the cleaning AND is redundant because the mask +// value is 32 bit width. +module bug5186(input wire clk, input wire [31:0] in, output out); + logic [63:0] d; + always_ff @(posedge clk) + d <= {d[31:0], in}; + + wire bad; + assign bad = {d[38:32], d[38] ^ d[31] ^ d[38]} != d[38:31]; + + logic result; + always_ff @ (posedge clk) + result <= bad; + assign out = result; +endmodule diff --git a/test_regress/t/t_const_opt_dfg.pl b/test_regress/t/t_const_opt_dfg.pl index 547adc392..3af0d6cd8 100755 --- a/test_regress/t/t_const_opt_dfg.pl +++ b/test_regress/t/t_const_opt_dfg.pl @@ -21,7 +21,7 @@ execute( ); if ($Self->{vlt}) { - file_grep($Self->{stats}, qr/Optimizations, Const bit op reduction\s+(\d+)/i, 37); + file_grep($Self->{stats}, qr/Optimizations, Const bit op reduction\s+(\d+)/i, 40); } ok(1); 1; diff --git a/test_regress/t/t_debug_width.out b/test_regress/t/t_debug_width.out index 0a1f7b907..5400328e1 100644 --- a/test_regress/t/t_debug_width.out +++ b/test_regress/t/t_debug_width.out @@ -1,4 +1,4 @@ -%Error: Internal Error: t/t_const_opt.v:531:34: ../V3Ast.cpp:#: widthMismatch detected 'lhsp()->widthMin() != rhsp()->widthMin()' @ ../V3AstNodes.cpp:#OUT:(G/wu32/1) LHS:(G/w32) RHS:(G/wu32/1) - 531 | always_ff @(posedge clkin_data[0], posedge myfirst, posedge mysecond) +%Error: Internal Error: t/t_const_opt.v:534:34: ../V3Ast.cpp:#: widthMismatch detected 'lhsp()->widthMin() != rhsp()->widthMin()' @ ../V3AstNodes.cpp:#OUT:(G/wu32/1) LHS:(G/w32) RHS:(G/wu32/1) + 534 | always_ff @(posedge clkin_data[0], posedge myfirst, posedge mysecond) | ^ ... See the manual at https://verilator.org/verilator_doc.html for more assistance.