Patch forthcoming.
Created attachment 335224 [details] the patch
Attachment 335224 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/air/AirCode.cpp:79: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/air/AirCode.cpp:80: More than one command on the same line [whitespace/newline] [4] Total errors found: 2 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 335224 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=335224&action=review > Source/JavaScriptCore/b3/air/AirCode.cpp:62 > + Vector<Reg> volatileRegs; Why not call this callerSaveRegs? Is it to avoid typo bugs? > Source/JavaScriptCore/b3/air/AirCode.cpp:65 > all.exclude(RegisterSet::stackRegisters()); I think the wasm bug might be that you are not excluding the pinned registers?
Comment on attachment 335224 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=335224&action=review >> Source/JavaScriptCore/b3/air/AirCode.cpp:65 >> all.exclude(RegisterSet::stackRegisters()); > > I think the wasm bug might be that you are not excluding the pinned registers? Nvm, I don't think this is the issue. Ignore me.
r=me.
(In reply to Keith Miller from comment #3) > Comment on attachment 335224 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335224&action=review > > > Source/JavaScriptCore/b3/air/AirCode.cpp:62 > > + Vector<Reg> volatileRegs; > > Why not call this callerSaveRegs? Is it to avoid typo bugs? Yeah. I don't really know what the best terminology is, but I have often used "volatile" versus "callee-save", because it's easier to distinguish the two. > > > Source/JavaScriptCore/b3/air/AirCode.cpp:65 > > all.exclude(RegisterSet::stackRegisters()); > > I think the wasm bug might be that you are not excluding the pinned > registers? Nah. This is just about order. The pinned regs will get excluded anyway.
Landed in https://trac.webkit.org/changeset/229412/webkit
<rdar://problem/38266059>
Comment on attachment 335224 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=335224&action=review > Source/WTF/wtf/MathExtras.h:534 > +template<typename VectorType, typename RandomFunc> > +void shuffleVector(VectorType& vector, size_t size, const RandomFunc& randomFunc) > +{ > + for (size_t i = 0; i + 1 < size; ++i) > + std::swap(vector[i], vector[i + randomFunc(size - i)]); > +} Why doesn’t this use std::random_shuffle?
(In reply to Darin Adler from comment #9) > Comment on attachment 335224 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335224&action=review > > > Source/WTF/wtf/MathExtras.h:534 > > +template<typename VectorType, typename RandomFunc> > > +void shuffleVector(VectorType& vector, size_t size, const RandomFunc& randomFunc) > > +{ > > + for (size_t i = 0; i + 1 < size; ++i) > > + std::swap(vector[i], vector[i + randomFunc(size - i)]); > > +} > > Why doesn’t this use std::random_shuffle? According to http://en.cppreference.com/w/cpp/algorithm/random_shuffle, std::random_shuffle is deprecated in C++14 and removed in C++17 so it probably doesn't make sense to use it. We could use std::shuffle but that has a really obnoxious API...