Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Division >64-bits fails on a 32-bit build #178

Closed
veripoolbot opened this issue Nov 10, 2009 · 4 comments
Closed

Division >64-bits fails on a 32-bit build #178

veripoolbot opened this issue Nov 10, 2009 · 4 comments
Assignees

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Nov 10, 2009


Author Name: Byron Bradley (@bbradley)
Original Redmine Issue: 178 from https://www.veripool.org
Original Date: 2009-11-10
Original Assignee: Wilson Snyder (@wsnyder)


Commit 8487d67 (not in a release yet) was tested on two CentOS 5.3 machines, one 32-bit the other 64-bit. test_regress/t/t_math_divw and test_regress/t/t_math_vgen pass on 64-bit but not on 32-bit. gcc "4.1.2 20080704 (Red Hat 4.1.2-44)" was being used on both machines. The common thing between these tests seems to be division of vectors >64-bit.

The output from both tests on the 32-bit machine is attached, it was run with --debug --verbose --gdbbt.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 10, 2009


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2009-11-10T13:55:09Z


Try this patch, it's the only thing that seems obviously wrong by inspection. If it doesn't work, try valgrind'ing it.

cd test_regress
valgrind /home/byronb/project/verilator/verilator/verilator_bin_dbg --batch --quiet -ex 'run --prefix Vt_math_divw --x-assign unique --debug --gdbbt -cc -Mdir obj_dir/t_math_divw --debug-check -f input.vc +define+TEST_VERBOSE=1 t/t_math_divw.v' -ex 'set width 0' -ex 'bt' -ex 'c' --no-dump-tree

diff --git a/src/V3Number.cpp b/src/V3Number.cpp
index 3a49a48..0024d79 100644
--- a/src/V3Number.cpp
+++ b/src/V3Number.cpp
@@ -1177,8 +1177,8 @@ V3Number& V3Number::opModDivGuts(const V3Number& lhs, const V3Number& rhs, bool
      uint32_t vn[VL_MULS_MAX_WORDS+1]; // v normalized

      // Zero for ease of debugging and to save having to zero for shifts
-    for (int i=0; i<6; i++) { un[i]=vn[i]=m_value[i]=0; }
-    for (int i=6; i<words+1; i++) { un[i]=vn[i]=0; }  // +1 as vn may get extra word
+    for (int i=0; i<words; i++) { m_value[i]=0; }
+    for (int i=0; i<words+1; i++) { un[i]=vn[i]=0; }  // +1 as vn may get extra word

      // Algorithm requires divisor MSB to be set
      // Copy and shift to normalize divisor so MSB of vn[vw-1] is set

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 10, 2009


Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2009-11-10T15:43:58Z


Brilliant, that fixes it. Thanks Wilson.

I have all of the regression tests passing although not just with a 'make test' or 'make v3'. t/t_sys_file_autoflush only passes if I run
t/t_sys_file_basic by hand first and t/t_case_write2 sometimes uses t_case_write1.out as the expected output. The patch below fixes the
t_case_write2 test but am I missing something else here?

diff --git a/test_regress/t/t_case_write1.pl b/test_regress/t/t_case_write1.pl
index ba24050..8ff03e2 100755
--- a/test_regress/t/t_case_write1.pl
+++ b/test_regress/t/t_case_write1.pl
@@ -7,7 +7,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
 # Lesser General Public License Version 3 or the Perl Artistic License
 # Version 2.0.
 
-$golden_out ||= "t/$Self->{name}.out";
+$Self->{golden_out} ||= "t/$Self->{name}.out";
 
 compile (
 	 v_flags2 => [$Self->{v3}?"--stats --O3 -x-assign fast":""],
@@ -17,5 +17,5 @@ execute (
 	 check_finished=>1,
       );
 
-ok(files_identical("$Self->{obj_dir}/$Self->{name}_logger.log", $golden_out));
+ok(files_identical("$Self->{obj_dir}/$Self->{name}_logger.log", $Self->{golden_out}));
 1;
diff --git a/test_regress/t/t_case_write2.pl b/test_regress/t/t_case_write2.pl
index 455859e..f19e4f4 100755
--- a/test_regress/t/t_case_write2.pl
+++ b/test_regress/t/t_case_write2.pl
@@ -7,7 +7,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
 # Lesser General Public License Version 3 or the Perl Artistic License
 # Version 2.0.
 
-$golden_out ||= "t/$Self->{name}.out";
+$Self->{golden_out} ||= "t/$Self->{name}.out";
 
 compile (
 	 v_flags2 => [$Self->{v3}?"--stats --O3 -x-assign fast":""],
@@ -17,6 +17,6 @@ execute (
 	 check_finished=>1,
       );
 
-ok(files_identical("$Self->{obj_dir}/$Self->{name}_logger.log", $golden_out));
+ok(files_identical("$Self->{obj_dir}/$Self->{name}_logger.log", $Self->{golden_out}));
 
 1;

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 10, 2009


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2009-11-10T15:56:47Z


Both and the autoflush fix (wrong directory mentioned) pushed to git.

BTW you can run tests in parallel if you have multicore;
cpan install Parallel::Forker and see test_regress/driver.pl.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Feb 7, 2010


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2010-02-07T12:41:17Z


In 3.800.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.