SIMD #4201

gcolvin · 2017-06-26T04:42:35Z

This is a portable implementation of the ethereum/EIPs#616 SIMD proposal.

The proposal itself has had a lot of attention as a result of trying to implement it, and because the wasm simd proposal surfaced.
The SIMD code itself is all in the new VMSIMD.cpp. The rest is just hooking it up.
The new opcodes and their implementations are walled off from production use with an EIP_616 macro set false in Instructions.h.
Yesterday I gave up on the fairly clean design I had been using - defining stack slots as a union of u256 and the simd vectors - because so much code changed accessing union members that rebasing failed miserably. The code became harder to read as well.
Instead I leave u256 as the slot type, so no code expecting that has to change. I punch through that type only in VMSIMD.cpp to store raw arrays on the same memory otherwise used by the u256 stack slots.

…mplemented remainder of simd ops

codecov-io · 2017-06-26T06:28:09Z

Codecov Report

Merging #4201 into develop will increase coverage by 0.19%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #4201      +/-   ##
===========================================
+ Coverage    67.35%   67.54%   +0.19%     
===========================================
  Files          302      302              
  Lines        23257    23363     +106     
===========================================
+ Hits         15664    15781     +117     
+ Misses        7593     7582      -11

Impacted Files	Coverage Δ
libevmcore/Instruction.cpp	`31.81% <ø> (ø)`	⬆️
libevmcore/Instruction.h	`100% <ø> (ø)`	⬆️
test/tools/fuzzTesting/fuzzHelper.cpp	`55.46% <ø> (+1.95%)`	⬆️
libevmcore/EVMSchedule.h	`100% <ø> (ø)`	⬆️
libevm/VM.h	`76.92% <ø> (ø)`	⬆️
test/unittests/libdevcrypto/LibSnark.cpp	`100% <100%> (ø)`	⬆️
libevm/VMCalls.cpp	`97.85% <100%> (+0.03%)`	⬆️
libevm/VM.cpp	`95.97% <100%> (+0.02%)`	⬆️
libethash/internal.c	`71.51% <0%> (-1.27%)`	⬇️
libp2p/Common.cpp	`52.8% <0%> (+0.79%)`	⬆️
... and 4 more

gcolvin · 2017-06-26T07:28:03Z

@winsvega clang won't compile the std::array initializer in fuzzHelper, but gcc and vcc will.

/Users/travis/build/ethereum/cpp-ethereum/test/tools/fuzzTesting/fuzzHelper.cpp:31:2: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]

C++11 requires extra braces, C++14 doesn't. So I added the silly braces. They didn't break gcc, will see soon if the heal clang.

… callvalue

snark: Add ECADD and ECMUL benchmark tests

#4200 (comment)

Fix STATICCALL

gumb0 · 2017-06-27T16:02:30Z

libevm/VM.cpp

@@ -14,8 +14,6 @@
 	You should have received a copy of the GNU General Public License
 	along with cpp-ethereum.  If not, see <http://www.gnu.org/licenses/>.
 */
-/** @file VM.cpp


I think we agreed on keeping these @file directives in the files

I think (could be wrong) Pawel asked me to remove them. I took a lot out.

@chriseth ?

gumb0 · 2017-06-27T16:11:11Z

libevm/VM.cpp

@@ -115,6 +113,22 @@ void VM::adjustStack(unsigned _removed, unsigned _added)
 #endif
 }

+void VM::sstoreGas()


Maybe name it updateGasForSstore

gumb0 · 2017-06-27T16:11:58Z

libevm/VM.cpp

@@ -115,6 +113,22 @@ void VM::adjustStack(unsigned _removed, unsigned _added)
 #endif
 }

+void VM::sstoreGas()
+{
+	if (m_ext->staticCall)


Better leave this check out of the function updating the gas, it't not related to gas in any way

I copied the existing code in SSTORE exactly. Could move this one piece to SSTORE and to XSTORE?

Yeah I should, it's included as a preamble to lots of functions that way. Somebody slipped them in while I wasn't looking ;)

gumb0 · 2017-06-27T16:14:54Z

libevm/VM.cpp

+			ON_OP();
+			updateIOGas();
+
+			xmul(m_code[++m_PC]); ++m_PC;


Better put each statement on the separate line for readability and easier debugging

Created one function for advancing the PC and fetching the simd type.

gumb0 · 2017-06-27T16:18:22Z

libevm/VM.cpp

+			ON_OP();
+			updateIOGas();
+
+			xoor(m_code[++m_PC]); ++m_PC;


There's VM::xoor and VM::xxor , shoudn't the latter be called here?
Also where's the case for XOOR ?

I think xoor is a typo and only one of them is needed, thanks.

Wrong. Seems xor is now a keyword, so I called the function xoor, and it needs a case to call it too.

gumb0 · 2017-06-27T16:55:39Z

libevm/VMSIMD.cpp

+
+void VM::vtow(uint8_t _b, const u256& _in, u256& _out)
+{
+	const uint8_t n = pow2N((_b) & 0xf), t = (_b) >> 4;


One declaration per line, according to coding standards

gumb0 · 2017-06-27T17:05:45Z

libevm/VMSIMD.cpp

+void VM::wtov(uint8_t _b, const u256& _in, u256& _out)
+
+{
+	const uint8_t n = pow2N((_b) & 0xf), t = (_b) >> 4;


This calculation is duplicated all over the place, create a couple of helper functions to get number of elements and type

gumb0 · 2017-06-27T17:12:59Z

libevm/VMSIMD.cpp

+typedef uint32_t a32x8 [8];
+typedef uint16_t a16x16[16];
+typedef uint8_t  a8x32 [32];
+      a64x4&  v64x4 (      u256& _stack_item) { return (a64x4&) *(a64x4*) &_stack_item; }


Declare these functions inline maybe?

Good idea. (Though compilers are pretty good now.)

gumb0 · 2017-06-27T17:28:45Z

libevm/VMSIMD.cpp

+	case 1:
+		for (int j = n, v = 0, i = 0; 0 < n; ++i)
+		{
+			v |= p[--j];


p[--j] is uint8_t, so you bit-or it with lowest 8 bits of v and then on the next line you shift right 8 bits, loosing what you've just read from p ? What am I getting wrong?

Looks like I pasted the mstore() code as a starting template and then failed to reverse the shifts. Thanks.

gumb0 · 2017-06-27T17:34:34Z

libevm/VMSIMD.cpp

+	switch (t)
+	{
+	case 0:
+		for (int i = 0; i < n; ++i) {


Braces on separate lines

Another bad habit. Messed up less often than usual in this code...

Yep, that one function looks to be the only place. A record!

gumb0 · 2017-06-27T17:38:35Z

This is an impressive amount of work, I wouldn't mind someone helping with the review 😃 @chfast @chriseth @pirapira

gumb0 · 2017-06-27T17:39:16Z

I haven't finished looking through all of the VMSIMD.cpp yet, will try to continue tomorrow.

So far I mostly have style-related comments and some clarifying questions.

gcolvin · 2017-06-27T18:45:34Z

Thanks @gumb0 Andrei

…mplemented remainder of simd ops

Fixes #4217.

This should improve performance.

gcolvin · 2017-07-07T04:45:05Z

This branch is gitted to death. Work has moved to /pull/4233.

gcolvin · 2017-07-10T00:59:08Z

Trashed and closed. Work continues here - #4233

chfast and others added 2 commits June 23, 2017 22:10

Fix STATICCALL giving stipend

d239486

Fixes #4199.

redesignd stack use to minimize impact of simd on remainer of code, i…

6702b18

…mplemented remainder of simd ops

gcolvin added the in progress label Jun 26, 2017

gcolvin requested review from chfast, chriseth and axic June 26, 2017 04:43

fuzzHelper wants 615 opcodes

f372161

Update fuzzHelper.cpp

18bc1c8

chfast and others added 6 commits June 26, 2017 10:13

Fix STATICCALL forwarding call value

cbe4b17

update tests

5bca159

snark: Add ECADD and ECMUL benchmark tests

f17ed62

Fix clang build issue

8a68de6

Update tests to include two new tests about staticcall not inheriting…

e232db4

… callvalue

Merge branch 'develop' into fix-staticcall

631c875

pirapira mentioned this pull request Jun 26, 2017

snark: Add ECADD and ECMUL benchmark tests #4178

Merged

pirapira and others added 4 commits June 26, 2017 17:16

Merge pull request #4178 from ethereum/snark-benchmarks

e2c2a0a

snark: Add ECADD and ECMUL benchmark tests

Update tests

53be05a

#4200 (comment)

Update tests

1809582

Merge pull request #4200 from ethereum/fix-staticcall

5ad949e

Fix STATICCALL

gumb0 reviewed Jun 27, 2017

View reviewed changes

gcolvin added 5 commits June 27, 2017 21:27

redesignd stack use to minimize impact of simd on remainer of code, i…

7d0fae1

…mplemented remainder of simd ops

fuzzHelper wants 615 opcodes

cdf9b02

Andrei's suggestions

65cf38b

conflicts

30b996c

commit now or comments later

a4fcfc9

pirapira and others added 24 commits July 5, 2017 09:26

Check names of GeneralStateTest test fillers against filenames

bbfeb5d

EVMJIT: STATICCALL

45dbc24

Fix Boost build on Gentoo

b242a2c

Fixes #4217.

travis_wait 80

d2201dd

travis_wait on brew install llvm, which takes more than 10 minutes

4405261

eth: Drop old interactive mode

a860a9d

logs: Simplify log output handling

25e009e

This should improve performance.

common: Remove SpinLock class

e17b1a5

eth: Remove obsolete command line options

d14407e

trace to logChannel in State and DB

efb99c0

general blockchain tests

4011184

bcExample

0f3f057

more debuginfo

939832c

bcForgedTest

4bfa739

fix style

47afcf2

fix GasPricer tests

883c8b4

limit mining threads to 1 for testeth

bf1d1dd

fix style issues

63fe8ba

correct --fillchain option for new blockchain fillers

7f78291

fixes and style update

c9b3bfe

change throw 1; to exit(1)

e803179

still trying to force github to accept push

9e487a6

yes, still trying to force github to accept push

67ab2a7

why am I still messing with this branch?

d2c2e80

gcolvin mentioned this pull request Jul 7, 2017

new branch for PR, attempted vcc fix, other cleanup #4233

Merged

gcolvin closed this Jul 10, 2017

gcolvin removed the in progress label Jul 19, 2017

gcolvin deleted the samd branch August 14, 2017 03:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SIMD #4201

SIMD #4201

gcolvin commented Jun 26, 2017 •

edited

Loading

codecov-io commented Jun 26, 2017 •

edited

Loading

gcolvin commented Jun 26, 2017

gumb0 Jun 27, 2017

gcolvin Jun 27, 2017

gcolvin Jun 29, 2017

gumb0 Jun 27, 2017

gumb0 Jun 27, 2017

gcolvin Jun 27, 2017

gcolvin Jun 27, 2017 •

edited

Loading

gumb0 Jun 27, 2017

gcolvin Jun 27, 2017

gumb0 Jun 27, 2017

gcolvin Jun 27, 2017

gcolvin Jun 27, 2017

gumb0 Jun 27, 2017

gumb0 Jun 27, 2017

gcolvin Jun 27, 2017

gumb0 Jun 27, 2017

gcolvin Jun 27, 2017

gumb0 Jun 27, 2017

gcolvin Jun 27, 2017

gcolvin Jul 4, 2017

gumb0 Jun 27, 2017

gcolvin Jun 27, 2017

gcolvin Jun 27, 2017

gumb0 commented Jun 27, 2017 •

edited

Loading

gumb0 commented Jun 27, 2017 •

edited

Loading

gcolvin commented Jun 27, 2017

gcolvin commented Jul 7, 2017

gcolvin commented Jul 10, 2017

SIMD #4201

SIMD #4201

Conversation

gcolvin commented Jun 26, 2017 • edited Loading

codecov-io commented Jun 26, 2017 • edited Loading

Codecov Report

gcolvin commented Jun 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gcolvin Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gumb0 commented Jun 27, 2017 • edited Loading

gumb0 commented Jun 27, 2017 • edited Loading

gcolvin commented Jun 27, 2017

gcolvin commented Jul 7, 2017

gcolvin commented Jul 10, 2017

gcolvin commented Jun 26, 2017 •

edited

Loading

codecov-io commented Jun 26, 2017 •

edited

Loading

gcolvin Jun 27, 2017 •

edited

Loading

gumb0 commented Jun 27, 2017 •

edited

Loading

gumb0 commented Jun 27, 2017 •

edited

Loading