Remove occasional redundant Push/Pop in FlexibleShift helper
Categories
(Core :: JavaScript Engine: JIT, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: mgaudet, Assigned: kellykc72, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=c++])
Attachments
(1 file)
In Bug 1526024 the FlexibleShift
helper was rewritten to fix correctness, with tests added.
A side effect of the new helper however is that when shift
is already in register ecx
, we'll generate a push/pop pair that are redundant, as that register already will not get clobbered.
This bug is about making that push conditional.
A patch for this bug should be done when it passes the jsapi-tests
unit test framework for subtests testJitMacroAssembler_flexible*shift
. A bonus would be if you used masm.breakpoint()
and a debugger to inspect the emitted instructions to verify the push and pop are elided.
Updated•5 years ago
|
(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #0)
In Bug 1526024 the
FlexibleShift
helper was rewritten to fix correctness, with tests added.A side effect of the new helper however is that when
shift
is already in registerecx
, we'll generate a push/pop pair that are redundant, as that register already will not get clobbered.This bug is about making that push conditional.
A patch for this bug should be done when it passes the
jsapi-tests
unit test framework for subteststestJitMacroAssembler_flexible*shift
. A bonus would be if you usedmasm.breakpoint()
and a debugger to inspect the emitted instructions to verify the push and pop are elided.
Hi, This was tagged 'Good-first-bug', I was hopping I could be assigned to it, but I do not see the 'Take' button. Please advice
Comment 2•5 years ago
|
||
Hi, thank you for expressing interest in working on this bug. For first bugs like this we usually like to keep them open in case someone disappears after expressing interest - but don't worry, it's just a formality!
To be honest, this might be more of a good second bug, as the change here does not sound trivial. But if you don't mind a challenge, I'd recommend getting started by checking out the build documentation. There's [1] for Firefox in general, and [2] for SpiderMonkey (the JavaScript engine) in particular. Good luck!
[1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Build_Documentation
(In reply to Emanuel Hoogeveen [:ehoogeveen] from comment #2)
Hi, thank you for expressing interest in working on this bug. For first bugs like this we usually like to keep them open in case someone disappears after expressing interest - but don't worry, it's just a formality!
To be honest, this might be more of a good second bug, as the change here does not sound trivial. But if you don't mind a challenge, I'd recommend getting started by checking out the build documentation. There's [1] for Firefox in general, and [2] for SpiderMonkey (the JavaScript engine) in particular. Good luck!
[1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Build_Documentation
Thank you very much, I'll take a look at these resources! Best Regards
(In reply to Emanuel Hoogeveen [:ehoogeveen] from comment #2)
Hi, thank you for expressing interest in working on this bug. For first bugs like this we usually like to keep them open in case someone disappears after expressing interest - but don't worry, it's just a formality!
To be honest, this might be more of a good second bug, as the change here does not sound trivial. But if you don't mind a challenge, I'd recommend getting started by checking out the build documentation. There's [1] for Firefox in general, and [2] for SpiderMonkey (the JavaScript engine) in particular. Good luck!
[1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Build_Documentation
Also, thank you for explaining :)
[Update:] Successfully built Firefox for Desktop. Proceeding to build SpiderMonkey
Reporter | ||
Comment 6•5 years ago
|
||
Hey kellykc72, sounds like you're started up. Let me know if you have questions.
This is indeed a slightly more challenging bug, but because it's in a test suite, I think it's not too bad compared to other JIT bugs :D
(And thanks so much Emanuel for jumping in on this!)
Haha, challenges are always fun. Just getting around to completing the SpiderMonkey build. I'll keep you all posted!
SpiderMonkey Build completed. I shall proceed to use searchfox to find the associated code. I'll ping back if i need more information. Thanks
Checking that inline void FlexibleShift32(MacroAssembler&, Register, Register, bool, bool)
is the function to be fixed. Is there a 64bit version? excuse me if that's a silly question.
Reporter | ||
Comment 10•5 years ago
|
||
You've got the right function!
Assignee | ||
Comment 11•5 years ago
|
||
Sweeet! I'll read up, get some context about the code, the bug what I'm thinking and you can verify and point me in the right direction. Thank you
Assignee | ||
Comment 12•5 years ago
|
||
Hey, so I wrapped preserve.add(ecx);
in a condition. However, I want to confirm that this statement from your original report "A patch for this bug should be done when it passes the jsapi-tests
unit test framework for subtests testJitMacroAssembler_flexible*shift
." means my fix must pass testJitMacroAssembler_flexible*shift
before it will be accepted.
Assignee | ||
Comment 13•5 years ago
|
||
In the meantime, I will research how to test and debug the JIT engine. Thank you
Reporter | ||
Comment 14•5 years ago
|
||
Yep. That's correct.
If you've not done so already, after you build the shell, there should be in dist/bin
a jsapi-tests
binary; you can run the shift tests by just doing ./dist/bin/jsapi-tests flexible
(ok, I mean, that also runs flexibleDivMod
and flexibleRemainder
but these don't take too long).
It might be cool/educational (up to you) to use masm.breakpoint()
to cause a trap to happen when the case you're fixing happens so you can look at the assembly code and see how it changes before and after your patch.
Assignee | ||
Comment 15•5 years ago
|
||
Oh nice, I actually came here to ask about what you just answered! So I found the binary but in other to find the usage, I was searching for an entry point. I saw a main
in tests.cpp
, but from this command you just gave ./dist/bin/jsapi-tests flexible
, it appears I had the wrong entry point. I guess the broader question will be 'are there any documentation on how the tests are structured/run?'
Oh yeah I was gonna do the breakpoint stuff. But before that. Please confirm my workflow, I feel its supposed to be a lot simpler.
- Modified the appropriate function
- Executed the
configure --enable-debug --disable-optimize --enable-nspr-build --enable-build-backends=CompileDB,RecursiveMake
I don't think I should have done this. Please confirm - Executed
mozmake
I think this is all I should have needed to do, although it seemed to have done way more stuff than I was expecting (considering I have run it before and only made a little change)
Let me go checkout the masm.breakpoint
stuff.
Assignee | ||
Comment 16•5 years ago
|
||
Also, could you assist me with the debugging workflow? I've added the debug code, I just don't know how to debug. I tried looking at https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Hacking_Tips
. Thanks a lot!
Assignee | ||
Comment 17•5 years ago
|
||
(In reply to kellykc72 from comment #16)
Also, please could you assist me with the debugging workflow? I've added the debug code, I just don't know how to debug. I tried looking at
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Hacking_Tips
. Thanks a lot!
Comment 18•5 years ago
|
||
Regarding the configure options, I don't think you need the --enable-build-backends
option. The others should be fine for debugging purposes - --disable-optimize
makes it slow, but without it a lot of things will get optimized out.
One thing to note: as of fairly recently, Windows shell builds are 64-bit by default (I assume you're on Windows because you mentioned mozmake
). If you're adjusting the x86 JIT backend you might need a cross-compile. Just adding --target=i686-pc-mingw32
might work, though if it complains about the host configuration being unknown you might need to add --host=x86_64-pc-mingw32
as well.
You can pass the -s
flag to mozmake
just like with make
to make it a bit more quiet, and -jn
with n
a number to make it perform multiple jobs in parallel (e.g. -j6
for 6 parallel jobs). The console output will get a bit mangled unfortunately, but it should be a lot faster.
Comment 19•5 years ago
|
||
Oh, and you might already be doing this, but it's highly recommended to build in a subdirectory. So you'd navigate to js/src
, mkdir mybuild
, cd mybuild
and then run configure as ../configure
instead.
Reporter | ||
Comment 20•5 years ago
|
||
(In reply to kellykc72 from comment #16)
Also, could you assist me with the debugging workflow? I've added the debug code, I just don't know how to debug. I tried looking at
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Hacking_Tips
. Thanks a lot!
(Bearing in mind I don't have a lot of Windows development experience)
It should be just a matter of attaching to jsapi-tests with a debugger; when you execute the JIT compiled code that has a masm.breakpoint()
in, most debuggers should stop there. At that point you're stopped in JIT compiled code, and so you don't get symbols or line numbers. You'd then want to check the assembly by using your debuggers disassembly instructions.
At this point it sounds like you've got a patch worth looking at: Follow the instructions here to submit a patch :D
Comment 21•5 years ago
|
||
Using Visual Studio, what you can do is go to File
, Open
, Project/Solution...
and open jsapi-tests.exe as though it were a project. Then you can adjust the project properties to set the arguments and click the Start
button to start debugging.
Alternatively you can try running devenv.exe
directly with the /DebugExe
parameter, but it might not be in your %PATH% (it's located in an awkward location like %ProgramFiles(x86)%\Microsoft Visual Studio\2017\Community\Common7\IDE
). If you want to try it, the correct incantation is devenv -DebugExe path/to/jsapi-tests.exe [parameters]
(the .exe
extension is required or -DebugExe
will silently fail).
Assignee | ||
Comment 22•5 years ago
|
||
Thanks [:ehoogeveen] and [:mgaudet]. Let me play around with these suggestions.
Assignee | ||
Comment 23•5 years ago
|
||
Hi Emanuel, thanks, using the VS17 debugger worked fine given the steps you illustrated!
Also, I didn't cross-compile yet. Do I have to? I didn't imagine there will be a drastic change in behavior, please advice.
Assignee | ||
Comment 24•5 years ago
|
||
Hi Matthew, I believe this is the behavior you are expecting? Please advice
shift in ECX (PUSH/POP pair ABSENT for ECX)
00000350A6DA0092 mov ecx,0FEEDBADh
00000350A6DA0097 mov eax,10h
00000350A6DA009C mov ecx,2
00000350A6DA00A1 int 3 :BREAK
EAX = 16, ECX = 2 // in DECIMAL
00000350A6DA00A2 shl eax,cl :16 << 2
EAX = 64, ECX = 2
00000350A6DA00A4 mov r11,0CFFB1FFA68h :*(0CFFB1FFA68h) is 64
EAX = 64, ECX = 2, R11 = 0CFFB1FFA68h, [0CFFB1FFA68h] = 64
00000350A6DA00AE cmp dword ptr [r11],eax :*(0CFFB1FFA68h) == 64 <=> Equal
EAX = 64, ECX = 2, R11 = 0CFFB1FFA68h, [0CFFB1FFA68h] = 64
00000350A6DA00B1 jne 00000350A6DA00CF :Jump not taken
EAX = 64, ECX = 2, R11 = 0CFFB1FFA68h, [0CFFB1FFA68h] = 64
00000350A6DA00B7 mov r11,0CFFB1FFA70h :*(0CFFB1FFA70h) is 2
EAX = 64, ECX = 2, R11 = 0CFFB1FFA70h, [0CFFB1FFA70h] = 2
00000350A6DA00C1 cmp dword ptr [r11],ecx :*(0CFFB1FFA70h) == 2 <=> Equal
EAX = 64, ECX = 2, R11 = 0CFFB1FFA70h, [0CFFB1FFA70h] = 2
00000350A6DA00C4 jne 00000350A6DA0338 :Jump not taken
EAX = 64, ECX = 2, R11 = 0CFFB1FFA70h, [0CFFB1FFA70h] = 2
00000350A6DA00CA jmp 00000350A6DA0F58 :Unconditional Jump taken
Random value in ECX (PUSH/POP pair PRESENT for ECX)
00000350A6DA0F58 mov ecx,0FEEDBADh
00000350A6DA0F5D mov eax,10h
00000350A6DA0F62 mov edx,2
00000350A6DA0F67 int 3 :BREAK
EAX = 16, EDX = 2, ECX = 0FEEDBADh
00000350A6DA0F68 push rcx :Save ECX (it'll be clobbered)
EAX = 16, EDX = 2, ECX = 0FEEDBADh
00000350A6DA0F69 mov rcx,rdx :Overwrite RCX content with RDX
EAX = 16, EDX = 2, ECX = 2
00000350A6DA0F6C shl eax,cl :16 << 2
EAX = 64, EDX = 2, ECX = 2
00000350A6DA0F6E pop rcx :Reset ECX using saved value
EAX = 64, EDX = 2, ECX = 0FEEDBADh
00000350A6DA0F6F mov r11,0CFFB1FFA68h :*(0CFFB1FFA68h) is 64
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 0CFFB1FFA68h
00000350A6DA0F79 cmp dword ptr [r11],eax :*(0CFFB1FFA68h) == 64 <=> Equal
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 0CFFB1FFA68h
00000350A6DA0F7C jne 00000350A6DA0FAD :Jump is not taken
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 0CFFB1FFA68h
00000350A6DA0F82 mov r11,0CFFB1FFA70h :*(0CFFB1FFA70h) is 2
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 0CFFB1FFA70h
00000350A6DA0F8C cmp dword ptr [r11],edx :*(0CFFB1FFA70h) == 2 <=> Equal
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 0CFFB1FFA70h
00000350A6DA0F8F jne 00000350A6DA1216 :Jump is not taken
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 0CFFB1FFA70h
00000350A6DA0F95 mov r11,7FF7F1F8ABE8h :*(7FF7F1F8ABE8h) is 0FEEDBADh
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 7FF7F1F8ABE8h
00000350A6DA0F9F cmp dword ptr [r11],ecx :*(7FF7F1F8ABE8h) == 0FEEDBADh <=> Equal
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 7FF7F1F8ABE8h
00000350A6DA0FA2 jne 00000350A6DA147F :Jump is not taken
EAX = 64, EDX = 2, ECX = 0FEEDBADh, R11 = 7FF7F1F8ABE8h
00000350A6DA0FA8 jmp 00000350A6DA1E33 :Unconditional Jump taken
Assignee | ||
Comment 25•5 years ago
|
||
(In reply to kellykc72 from comment #23)
Hi Emanuel, thanks, using the VS17 debugger worked fine given the steps you illustrated!
Also, I didn't cross-compile yet. Do I have to? I didn't imagine there will be a drastic change in behavior, please advice.
Actually I tried to compile with those options and it ended in chaos haha
Assignee | ||
Comment 26•5 years ago
|
||
(In reply to kellykc72 from comment #25)
(In reply to kellykc72 from comment #23)
Hi Emanuel, thanks, using the VS17 debugger worked fine given the steps you illustrated!
Also, I didn't cross-compile yet. Do I have to? I didn't imagine there will be a drastic change in behavior, please advice.Actually I tried to configure with those options and it ended in chaos haha
I'm probably missing something but ...
In file included from d:/firefox/js/src\jit/LIR.h:1806d:/firefox/js/src\jit/LIR.h:
(715,d:/firefox/js/src\jit/x86/LIR-x86.h(209,6314)):: noteerror: : expanded fromno macromember 'LIR_HEADER'named
'WasmAtomicStoreI64' static constexpr LNode::Opcode classOpcode = LNode::Opcode::opcode;
in 'js::jit::LNode::Opcode' ~~~~~~~~~~~~~~~^
LIR_HEADER(WasmAtomicStoreI64);
~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
d:/firefox/js/src\jit/LIR.h(715,63): note: expanded from macro 'LIR_HEADER'
static constexpr LNode::Opcode classOpcode = LNode::Opcode::opcode;
~~~~~~~~~~~~~~~^
In file included from d:/firefox/js/src/build_DBG.OBJ/js/src/wasm/Unified_cpp_js_src_wasm1.cpp:47:
In file included from d:/firefox/js/src/wasm/WasmIonCompile.cpp:23:
In file included from d:/firefox/js/src\jit/CodeGenerator.h:16:
In file included from d:/firefox/js/src\jit/x86/CodeGenerator-x86.h:10:
In file included from d:/firefox/js/src\jit/x86-shared/CodeGenerator-x86-shared.h:10:
In file included from d:/firefox/js/src\jit/shared/CodeGenerator-shared.h:16:
In file included from d:/firefox/js/src\jit/LIR.h:1806:
d:/firefox/js/src\jit/x86/LIR-x86.h(284,14): error: no member named 'WasmAtomicBinopI64' in 'js::jit::LNode::Opcode'
LIR_HEADER(WasmAtomicBinopI64);
~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
d:/firefox/js/src\jit/LIR.h(715,63): note: expanded from macro 'LIR_HEADER'
static constexpr LNode::Opcode classOpcode = LNode::Opcode::opcode;
~~~~~~~~~~~~~~~^
In file included from d:/firefox/js/src/vm/Interpreter.cpp:56:
In file included from d:/firefox/js/src\jit/JitFrames-inl.h:12:
In file included from d:/firefox/js/src\jit/LIR.h:1806:
d:/firefox/js/src\jit/x86/LIR-x86.h(233,14): error: no member named 'WasmCompareExchangeI64' in 'js::jit::LNode::Opcode'
LIR_HEADER(WasmCompareExchangeI64);
~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
d:/firefox/js/src\jit/LIR.h(715,63): note: expanded from macro 'LIR_HEADER'
static constexpr LNode::Opcode classOpcode = LNode::Opcode::opcode;
~~~~~~~~~~~~~~~^
In file included from d:/firefox/js/src/vm/Interpreter.cpp:56:
In file included from d:/firefox/js/src\jit/JitFrames-inl.h:12:
In file included from d:/firefox/js/src\jit/LIR.h:1806:
d:/firefox/js/src\jit/x86/LIR-x86.h(261,14): error: no member named 'WasmAtomicExchangeI64' in 'js::jit::LNode::Opcode'
LIR_HEADER(WasmAtomicExchangeI64);
~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
d:/firefox/js/src\jit/LIR.h(715,63): note: expanded from macro 'LIR_HEADER'
static constexpr LNode::Opcode classOpcode = LNode::Opcode::opcode;
~~~~~~~~~~~~~~~^
In file included from d:/firefox/js/src/vm/Interpreter.cpp:56:
In file included from d:/firefox/js/src\jit/JitFrames-inl.h:12:
In file included from d:/firefox/js/src\jit/LIR.h:1806:
d:/firefox/js/src\jit/x86/LIR-x86.h(284,14): error: no member named 'WasmAtomicBinopI64' in 'js::jit::LNode::Opcode'
LIR_HEADER(WasmAtomicBinopI64);
~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
d:/firefox/js/src\jit/LIR.h(715,63): note: expanded from macro 'LIR_HEADER'
static constexpr LNode::Opcode classOpcode = LNode::Opcode::opcode
Assignee | ||
Comment 27•5 years ago
|
||
Oh I forgot the test output
$ ./dist/bin/jsapi-tests.exe flexible
testJitMacroAssembler_flexibleLshift
TEST-PASS | testJitMacroAssembler_flexibleLshift | ok
testJitMacroAssembler_flexibleRshiftArithmetic
TEST-PASS | testJitMacroAssembler_flexibleRshiftArithmetic | ok
testJitMacroAssembler_flexibleRshift
TEST-PASS | testJitMacroAssembler_flexibleRshift | ok
testJitMacroAssembler_flexibleQuotient
TEST-PASS | testJitMacroAssembler_flexibleQuotient | ok
testJitMacroAssembler_flexibleRemainder
TEST-PASS | testJitMacroAssembler_flexibleRemainder | ok
testJitMacroAssembler_flexibleDivMod
TEST-PASS | testJitMacroAssembler_flexibleDivMod | ok
Passed: ran 6 tests.
Reporter | ||
Comment 28•5 years ago
|
||
This code is shared between x86 and x86-64 backends, so no need to cross compile!
Reporter | ||
Comment 29•5 years ago
|
||
I think the behaviour looks right. Definitely time to submit a patch :)
Assignee | ||
Comment 30•5 years ago
|
||
sweeet!!
Comment 31•5 years ago
|
||
FWIW cross compilation seems to be broken for me at the moment, at least using mozmake and configure (mach seems okay). I'll have to file a bug, but no need to worry about it here.
Assignee | ||
Comment 32•5 years ago
|
||
Update: busy week(end), just getting around setting up Phabricator
. Didn't know earlier, should've set it up with everything else before starting. Sorry for the delay
Assignee | ||
Comment 33•5 years ago
|
||
The ECX register must contain the SHIFT amount. Before this change, ECX will always be push/pop during a SHIFT operation. However, ECX should only be push/pop if it's original contents are going to be overwritten. We now only push/pop ECX if it contains some arbituary value and hence will be overwritten by the SHIFT operation.
Updated•5 years ago
|
Reporter | ||
Comment 34•5 years ago
|
||
Thanks so much for your contribution! If you'd like to keep rolling, please, feel free to look for more mentored bugs in SpiderMonkey.
Perhaps give Bug 1531479 a look.
(Pulsebot would normally link to the landed code, but it seems to be sleeping right now, so:
https://hg.mozilla.org/integration/autoland/rev/5ec94981916f0660a2cd2348ef41a10ffc4fca93
)
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 35•5 years ago
|
||
(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #34)
Thanks so much for your contribution! If you'd like to keep rolling, please, feel free to look for more mentored bugs in SpiderMonkey.
Perhaps give Bug 1531479 a look.
(Pulsebot would normally link to the landed code, but it seems to be sleeping right now, so:
https://hg.mozilla.org/integration/autoland/rev/5ec94981916f0660a2cd2348ef41a10ffc4fca93
)
Thanks to you and :ehoogeveen for all the assistance and yes, I'll keep rolling. Let me take a look :)
Assignee | ||
Comment 36•5 years ago
|
||
Hi, please confirm no further action's are required from me for this. I read something about landing but I suppose you as the reviewer is responsible for that?
Assignee | ||
Comment 37•5 years ago
|
||
Oh nvm, I guess that's the link you sent regarding Pulsebot. Thanks!
Reporter | ||
Comment 38•5 years ago
|
||
You're all done. What's happening now is the code has landed on our integration branch 'autoland'; it's been tested through our continuous integration system; you can see the results here.
Next up, at some point in the not too distant future the Mozilla Sheriffs will merge autoland into mozilla-central, the repository from which our nightly builds are built. At that point, this bug will get marked as RESOLVED by the sheriff at the merge time.
Then, a few hours after that your code will be in a copy of Firefox Nightly that tens of thousands run every day :)
Party time: 🎉🎉🎉
Assignee | ||
Comment 39•5 years ago
|
||
(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #38)
You're all done. What's happening now is the code has landed on our integration branch 'autoland'; it's been tested through our continuous integration system; you can see the results here.
Next up, at some point in the not too distant future the Mozilla Sheriffs will merge autoland into mozilla-central, the repository from which our nightly builds are built. At that point, this bug will get marked as RESOLVED by the sheriff at the merge time.
Then, a few hours after that your code will be in a copy of Firefox Nightly that tens of thousands run every day :)
Party time: 🎉🎉🎉
Haha, thanks for the clarification! Already looking forward to my next contribution!
Comment 40•5 years ago
|
||
(In reply to kellykc72 from comment #35)
Thanks to you and :ehoogeveen for all the assistance and yes, I'll keep rolling. Let me take a look :)
No problem, congrats on your first contribution! I'm glad you had a positive experience and I'm looking forward to seeing what you do in the future :)
Comment 41•5 years ago
|
||
bugherder |
Comment 42•5 years ago
|
||
Description
•