Fix wrong bit op tree optimization (#3185)

* Add a test to reproduce bug3182. Run the same HDL with -Oo to confirm the result is same.

* Hopefully fix #3182. The result can be 0 only when polarity is true (no AstNot is found during traversal).
This commit is contained in:
Yutetsu TAKATSUKASA 2021-11-06 12:31:50 +09:00 committed by GitHub
parent e69a8e838d
commit b08c694cd6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 65 additions and 4 deletions

View File

@ -537,7 +537,7 @@ class ConstBitOpTreeVisitor final : public AstNVisitor {
if (leafInfo.m_lsb < leafInfo.width()) {
m_bitPolarities.emplace_back(leafInfo, isXorTree() || leafInfo.m_polarity,
leafInfo.m_lsb);
} else if (isAndTree()) {
} else if (isAndTree() && leafInfo.m_polarity) {
// If there is a constant 0 term in an And tree, we must include it. Fudge
// this by adding a bit with both polarities, which will simplify to zero
m_bitPolarities.emplace_back(leafInfo, true, 0);

View File

@ -0,0 +1,24 @@
#!/usr/bin/env perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2003 by Wilson Snyder. 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(simulator => 1);
top_filename("t/t_const_opt.v");
# Run the same design as t_const_opt.pl without bitopt tree optimization to make sure that the result is same.
compile(
verilator_flags2 => ["-Wno-UNOPTTHREADS", "--stats", "-Oo", "$Self->{t_dir}/t_const_opt.cpp"],
);
execute(
check_finished => 1,
);
ok(1);
1;

View File

@ -0,0 +1,13 @@
// -*- mode: C++; c-file-style: "cc-mode" -*-
//*************************************************************************
//
// Copyright 2021 by Yutetsu TAKATSUKASA. 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
//
//*************************************************************************
// This function is used to introduce dependency and disturb optimization
extern "C" int fake_dependency() { return 0; }

View File

@ -11,7 +11,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
scenarios(simulator => 1);
compile(
verilator_flags2 => ["-Wno-UNOPTTHREADS", "--stats"],
verilator_flags2 => ["-Wno-UNOPTTHREADS", "--stats", "$Self->{t_dir}/$Self->{name}.cpp"],
);
execute(

View File

@ -57,7 +57,7 @@ module t(/*AUTOARG*/
$write("[%0t] cyc==%0d crc=%x sum=%x\n",$time, cyc, crc, sum);
if (crc !== 64'hc77bb9b3784ea091) $stop;
// What checksum will we end up with (above print should match)
`define EXPECTED_SUM 64'he78be35df15ae0ab
`define EXPECTED_SUM 64'ha916d9291821c6e0
if (sum !== `EXPECTED_SUM) $stop;
$write("*-* All Finished *-*\n");
$finish;
@ -77,10 +77,11 @@ module Test(/*AUTOARG*/
input [31:0] i;
logic [31:0] d;
logic d0, d1, d2, d3, d4, d5, d6, d7;
logic bug3182_out;
output logic o;
logic [4:0] tmp;
logic [5:0] tmp;
assign o = ^tmp;
always_ff @(posedge clk) begin
@ -101,6 +102,29 @@ module Test(/*AUTOARG*/
tmp[2] <= ((d0 & d1) | (d0 & d2))^ ((d3 & d4) | (d5 & d4)); // replaceAndOr()
tmp[3] <= d0 <-> d1; // replaceLogEq()
tmp[4] <= i[0] & (i[1] & (i[2] & (i[3] | d[4]))); // ConstBitOpTreeVisitor::m_frozenNodes
tmp[5] <= bug3182_out;
end
bug3182 i_bug3182(.in(d[4:0]), .out(bug3182_out));
endmodule
module bug3182(in, out);
input wire [4:0] in;
output wire out;
// This function always returns 0, so safe to take bitwise OR with any value.
// Calling this function stops constant folding as Verialtor does not know
// what this function returns.
import "DPI-C" context function int fake_dependency();
logic [4:0] bit_source;
/* verilator lint_off WIDTH */
always @(in)
bit_source = fake_dependency() | in;
wire [5:0] tmp = bit_source; // V3Gate should inline this
wire out = ~(tmp >> 5) & (bit_source == 5'd10);
/* verilator lint_on WIDTH */
endmodule