Skip to content

Commit

Permalink
Revert changes on memcpy matrix getters
Browse files Browse the repository at this point in the history
Fix issue Qucs#133

The C memcpy was replaced by loops, but it causes the kernel to segfault
during SP analysis with Noise.

memcpy is good with POD and tends to be inlined [1].
Replacement candidate is std:copy. Please provide testcase and
testbench.

[1] http://nadeausoftware.com/articles/2012/05/c_c_tip_how_copy_memory_quickly
  • Loading branch information
guitorri committed Nov 28, 2014
1 parent f13d166 commit eb461ea
Showing 1 changed file with 3 additions and 9 deletions.
12 changes: 3 additions & 9 deletions qucs-core/src/circuit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -653,9 +653,7 @@ void circuit::setMatrixS (matrix s) {
circuit. */
matrix circuit::getMatrixS (void) {
matrix res (size);
for(unsigned int i=0; i < size; ++i)
for(unsigned int j=0; i < size; ++j)
res(i,j) = MatrixS[i*size + j];
memcpy (res.getData (), MatrixS, sizeof (nr_complex_t) * size * size);
return res;
}

Expand All @@ -674,9 +672,7 @@ void circuit::setMatrixN (matrix n) {
matrix of the circuit. */
matrix circuit::getMatrixN (void) {
matrix res (size);
for(unsigned int i=0; i < size; ++i)
for(unsigned int j=0; i < size; ++j)
res(i,j) = MatrixN[i*size + j];
memcpy (res.getData (), MatrixN, sizeof (nr_complex_t) * size * size);
return res;
}

Expand All @@ -695,9 +691,7 @@ void circuit::setMatrixY (matrix y) {
circuit. */
matrix circuit::getMatrixY (void) {
matrix res (size);
for(unsigned int i=0; i < size; ++i)
for(unsigned int j=0; i < size; ++j)
res(i,j) = MatrixY[i*size + j];
memcpy (res.getData (), MatrixY, sizeof (nr_complex_t) * size * size);
return res;
}

Expand Down

1 comment on commit eb461ea

@in3otd
Copy link

@in3otd in3otd commented on eb461ea Nov 28, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh, it took me a long time staring at the screen to realize that the problem was the check in the inner for loop, that should be j < size instead of i < size...

FYI, I have tried both version, with memcpy and the for loops and I do not see any difference in the simulation time for a moderately complex circuit.

I also compiled with clang++ and g++ to see if there was a significant difference between the two:
when using clang++ the simulation time was
real 0m23.195s using -O0
real 0m7.668s using -O2

when using g++, the time was
real 0m9.963s using -O2
real 0m6.372s using -Ofast, etc.

so clang++ is working quite well...

Please sign in to comment.