Hello, I was wondering, I think maybe Turbofan is already doing Smi checking/necessary conversions?
Here is a javascript file I wrote, switch2.js, and the output.txt (https://drive.google.com/file/d/12Zz4kznIesa1RzxCnCKPLXPmsuSrOGSZ/view?usp=sharing) - Run with --print-code and --print-bytecode turned on. - I also verified that sparkplug is not running in this case. - The file has the bytecode generated at the front, and the source of switch2.js on line 85 or so - switch2.js is verified to call the BytecodeGraphBuilder method (I put a print statement to verify) - I noticed as well it is optimized and presumably deoptimized several times when run with more iterations than 6000 currently in source, not sure if this is relevant information (maybe because i is switching between an integer-representable type and back again) The output looks like Smi conversion is happening (line 200 in output.txt, the ucomisd and vcvttsd2si instructions are happening, before the indirect jump with the jump table). I think this is not from any sparkplug code I already wrote, since that was verified as not running. I am guessing it is collecting type feedback maybe? I was wondering whether there was another use case I was missing that I should be testing on, since I tried a couple cases like this, and all seemed to work with no further modifications, so I wasn't sure. Thank you! On Monday, July 19, 2021 at 2:50:29 AM UTC-5 [email protected] wrote: > Correct, x86, x64, arm and arm64 are all you need to do. > > On Mon, Jul 19, 2021 at 6:40 AM Mihir Shah <[email protected]> wrote: > >> Hi, >> >> Just to update, so I implemented the sparkplug code for the x86/arm >> machines for 32-bit and 64-bit; I wasn't sure did I need to support MIPS or >> any of the other architectures, since the Handling of ports · V8 >> <https://v8.dev/docs/ports> link says that MIPS is not officially >> supported? >> >> Thank you! >> >> On Tuesday, July 13, 2021 at 12:21:00 AM UTC-5 Mihir Shah wrote: >> >>> Hi, >>> >>> So I was implementing the baseline compiler routine to do the Smi >>> conversion, and I just wanted to make sure I was on the right track: >>> Here is a short document outlining what I am doing, whenever it is >>> possible can you please look through this? >>> >>> https://docs.google.com/document/d/1hev0vI7o7gjD85fUkZVYcg3rt0guZ_bk6lNkHsSkt-w/edit?usp=sharing >>> >>> >>> >>> Thank you! >>> >>> On Friday, July 9, 2021 at 3:10:16 AM UTC-5 [email protected] wrote: >>> >>>> There's almost certainly cases in the runtime where we don't normalize >>>> a HeapNumber to a Smi -- my comment on floats representable as Smis being >>>> normalized to Smis was only for the numbers generated by the parser. >>>> >>>> On Thu, Jul 8, 2021 at 9:20 PM Mihir Shah <[email protected]> wrote: >>>> >>>>> Hi, >>>>> >>>>> Also I had another question, I noticed in the >>>>> TryTaggedToInt32AsIntPtr() method, in the case it is not an Smi but is a >>>>> Heap number, we load the float64, truncate to an int32, and then do the >>>>> strict equality check between the original and truncated version to make >>>>> sure the new form is equivalent. >>>>> >>>>> I was wondering, in what case would the float64 ever be equivalent to >>>>> an int32, except for -0.0? (Because I remember like we were saying that >>>>> any >>>>> float representable as an Smi will be stored as an Smi, back when we were >>>>> doing the switch case analysis) In this case, I was wondering if we could >>>>> streamline the logic just to check for a -0.0? >>>>> >>>>> Thank you! >>>>> >>>>> On Wednesday, July 7, 2021 at 10:54:53 AM UTC-5 [email protected] >>>>> wrote: >>>>> >>>>>> v8-dev to bcc >>>>>> >>>>>> It's easier to discuss this further in the doc, but basically in CSA >>>>>> code you can mark a rarely-taken label as "deferred" to move that chunk >>>>>> of >>>>>> code to the end of the generated code. This makes the fast-path of not >>>>>> taking the label faster. >>>>>> >>>>>> On Wed, Jul 7, 2021 at 4:43 PM Mihir Shah <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Thank you for this feedback, I will go back to the old method then. >>>>>>> Also just to follow up, in one of your comment you mentioned, "We >>>>>>> could even consider making the remaining conversion code "deferred" so >>>>>>> that >>>>>>> it doesn't fall out of line." I wasn't understanding what would this >>>>>>> look >>>>>>> like? >>>>>>> >>>>>>> Thank you! >>>>>>> >>>>>>> On Tuesday, July 6, 2021 at 10:45:01 PM UTC-5 Mihir Shah wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> Sorry to bug you again, but I was thinking maybe if we eliminated >>>>>>>> the extra bytecodes that could potentially reduce optimize::duration, >>>>>>>> since >>>>>>>> the optimizer has to handle/optimize on more than a dozen bytecodes >>>>>>>> extra. >>>>>>>> I created a design doc for preliminaries on porting (which we were >>>>>>>> planning >>>>>>>> on doing later I think anyway), >>>>>>>> https://docs.google.com/document/d/1Nu5dBnmoL7IcFI-ZPvtjhk97rqu8B6m9IgQgQXLgPl4/edit?resourcekey=0-L-yAxQwVqRlKtIXmomJ1-g, >>>>>>>> >>>>>>>> can you please let me know if this sounds good? >>>>>>>> >>>>>>>> Thank you! >>>>>>>> >>>>>>>> On Monday, July 5, 2021 at 6:01:55 AM UTC-5 [email protected] >>>>>>>> wrote: >>>>>>>> >>>>>>>>> (v8-dev to bcc) >>>>>>>>> >>>>>>>>> I replied on the bug, basically I imagine that Switch nodes aren't >>>>>>>>> well supported by the optimizer -- either because of node traversal >>>>>>>>> costs, >>>>>>>>> or because of optimization passes being designed only for Branch. >>>>>>>>> >>>>>>>>> On Wed, Jun 30, 2021 at 5:30 PM Mihir Shah <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Hi, sorry to bug you but just to follow up, I was not >>>>>>>>>> understanding why creating a chain of branches would improve upon a >>>>>>>>>> large >>>>>>>>>> switch node ( 1223133 - [V8 Perf Sheriff]: 3 regressions in >>>>>>>>>> v8.browsing_desktop - chromium >>>>>>>>>> <https://bugs.chromium.org/p/chromium/issues/detail?id=1223133#c9>), >>>>>>>>>> in improving the optimize duration performance? >>>>>>>>>> Thank you! >>>>>>>>>> On Thursday, June 24, 2021 at 5:35:53 AM UTC-5 >>>>>>>>>> [email protected] wrote: >>>>>>>>>> >>>>>>>>>>> Hi Mihir, >>>>>>>>>>> >>>>>>>>>>> Thanks for your contribution! I think you have two good options >>>>>>>>>>> for followups: >>>>>>>>>>> >>>>>>>>>>> a) Port your switch statement prologue from being in bytecodes >>>>>>>>>>> to being in Sparkplug and TurboFan (effectively, the first version >>>>>>>>>>> we >>>>>>>>>>> almost submitted, except working well with the other compilers). >>>>>>>>>>> >>>>>>>>>>> b) As you say, this Smi variable thing. There's no bug for it, >>>>>>>>>>> I think it's actually quite an original idea from your side. This >>>>>>>>>>> sort of >>>>>>>>>>> thing would do well with a design doc first, specifying more >>>>>>>>>>> precisely what >>>>>>>>>>> sort of variables you'd be able to inline (my guess would be >>>>>>>>>>> never-assigned >>>>>>>>>>> vars with Literal initializers), and what the consequences would be >>>>>>>>>>> (e.g. >>>>>>>>>>> the additional memory cost of repeated LdaSmi, smaller stack frames >>>>>>>>>>> because >>>>>>>>>>> there's fewer registers, potential for constant folding(?)). I >>>>>>>>>>> actually >>>>>>>>>>> have no idea if this would be a net benefit or not, so it'd be good >>>>>>>>>>> to have >>>>>>>>>>> an analysis. You could follow the template in our other design >>>>>>>>>>> docs, e.g. >>>>>>>>>>> https://docs.google.com/document/d/1qR_C8qYdKsDQFbFliAZLw2zmZ0nhA0xUCld1ba3N2Zk/edit#heading=h.n63k76b3zfwa >>>>>>>>>>> >>>>>>>>>>> - Leszek >>>>>>>>>>> >>>>>>>>>>> On Wed, Jun 23, 2021 at 5:45 PM Mihir Shah <[email protected]> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi, thank you for all the feedback on the PR, I learnt a lot >>>>>>>>>>>> from the process! >>>>>>>>>>>> >>>>>>>>>>>> Also I was interested in doing another project, I remember you >>>>>>>>>>>> had mentioned that opportunistic inlining of Smi variables/eliding >>>>>>>>>>>> it >>>>>>>>>>>> entirely (Commit message · Gerrit Code Review >>>>>>>>>>>> (googlesource.com) >>>>>>>>>>>> <https://chromium-review.googlesource.com/c/v8/v8/+/2904926/2..20//COMMIT_MSG#b19>) >>>>>>>>>>>> >>>>>>>>>>>> could be a good future project. >>>>>>>>>>>> >>>>>>>>>>>> I was wondering whether you think this would be a good next >>>>>>>>>>>> project for me to work on, and if so, I was wondering what would >>>>>>>>>>>> be the >>>>>>>>>>>> acceptance criteria for such a PR (since I can't find an issue on >>>>>>>>>>>> the v8 >>>>>>>>>>>> issue tracker with a similar purpose)? >>>>>>>>>>>> >>>>>>>>>>>> Thank you! >>>>>>>>>>>> On Monday, May 17, 2021 at 6:35:23 AM UTC-5 [email protected] >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> I actually meant implementing a Smi check in the bytecode >>>>>>>>>>>>> handler itself, in interpreter-generator.cc >>>>>>>>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:v8/src/interpreter/interpreter-generator.cc;l=2237;drc=cc06b8c77808f6da28919a88c2d7b25fbc327b8b>, >>>>>>>>>>>>> >>>>>>>>>>>>> not as additional bytecodes. >>>>>>>>>>>>> >>>>>>>>>>>>> On Mon, May 17, 2021 at 2:55 AM Mihir Shah < >>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi, so I implemented an Smi check by checking (x|0) === x, >>>>>>>>>>>>>> (since I was not able to get subtracting 0 to work with some of >>>>>>>>>>>>>> the special >>>>>>>>>>>>>> case in test/mjstest/switch.js like feeding a NaN or a number >>>>>>>>>>>>>> very close to >>>>>>>>>>>>>> an integer case). If the equality check fails, then I jump to >>>>>>>>>>>>>> the default >>>>>>>>>>>>>> case or after the switch, depending on whether there is a >>>>>>>>>>>>>> default case. >>>>>>>>>>>>>> >>>>>>>>>>>>>> This passes all the test cases in release mode, but in debug >>>>>>>>>>>>>> mode, the assertion in the SwitchOnSmi in >>>>>>>>>>>>>> src/interpreter/interpreter_generator.cc that checks whether the >>>>>>>>>>>>>> tag of the >>>>>>>>>>>>>> switch is an Smi fails for certain malformed inputs to the >>>>>>>>>>>>>> switch (when >>>>>>>>>>>>>> running the switch.js test). >>>>>>>>>>>>>> >>>>>>>>>>>>>> I was not sure how I could get around this, since I did not >>>>>>>>>>>>>> see any convert to Smi bytecode. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thank you! >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Saturday, May 15, 2021 at 11:12:29 AM UTC-5 Mihir Shah >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Oh ok I will add the smi check then, thank you! >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Saturday, May 15, 2021 at 1:45:42 AM UTC-5 >>>>>>>>>>>>>>> [email protected] wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> You could save a register and a TestEqual if you did two >>>>>>>>>>>>>>>> jumps (one for x<a, one for x>=b) but otherwise there's not >>>>>>>>>>>>>>>> much you can do >>>>>>>>>>>>>>>> for a range check. Keep in mind though that SwitchOnSmi does a >>>>>>>>>>>>>>>> range check >>>>>>>>>>>>>>>> itself (and falls through on failure), so unless you want to >>>>>>>>>>>>>>>> exclude a >>>>>>>>>>>>>>>> range that's inside your Switch range, you don't need a range >>>>>>>>>>>>>>>> check. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Another thing to keep in mind is that SwitchOnSmi expects a >>>>>>>>>>>>>>>> Smi value in the accumulator, so you'll need to add a Smi >>>>>>>>>>>>>>>> check to the >>>>>>>>>>>>>>>> bytecode handler. Thankfully 'switch' comparison semantics are >>>>>>>>>>>>>>>> that of >>>>>>>>>>>>>>>> strict equality, so afaict you don't need to add a ToPrimitive >>>>>>>>>>>>>>>> or anything >>>>>>>>>>>>>>>> like that, but I think you might have to add an explicit -0 >>>>>>>>>>>>>>>> check before >>>>>>>>>>>>>>>> the Smi check. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Hope that helps, happy to review when you get a patch >>>>>>>>>>>>>>>> together - this has been on my backlog for literal years, so >>>>>>>>>>>>>>>> I'm glad to >>>>>>>>>>>>>>>> see someone else doing it! >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> - Leszek >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Sat, 15 May 2021, 07:03 Mihir Shah, < >>>>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> I was working on a jump table implementation for switch >>>>>>>>>>>>>>>>> statements with all constant Smi case labels (in the parse >>>>>>>>>>>>>>>>> step, instead of >>>>>>>>>>>>>>>>> generating the traditional if-else-if kind of logic), and >>>>>>>>>>>>>>>>> needed to do >>>>>>>>>>>>>>>>> range checking at the top. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> I was wondering, then, was there a better way to do range >>>>>>>>>>>>>>>>> checking, i.e. does value in accumulator register x lie in >>>>>>>>>>>>>>>>> range (known at >>>>>>>>>>>>>>>>> compile time) [a,b]? I think the standard trick of reducing >>>>>>>>>>>>>>>>> a<=x<=b to >>>>>>>>>>>>>>>>> 0<=x-a<=b-a and then doing unsigned comparison here would not >>>>>>>>>>>>>>>>> work because >>>>>>>>>>>>>>>>> Smi is signed. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Because right now my idea of the bytecode is something >>>>>>>>>>>>>>>>> like this (which feels very inefficient): >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Load x into accumulator and register r1 ... >>>>>>>>>>>>>>>>> TestLessThan [b] >>>>>>>>>>>>>>>>> Star [r2] >>>>>>>>>>>>>>>>> Ldar [r1] >>>>>>>>>>>>>>>>> TestGreaterThan [a] >>>>>>>>>>>>>>>>> TestEqual [r2] >>>>>>>>>>>>>>>>> JumpIfFalse <after the switch> >>>>>>>>>>>>>>>>> Ldar [r1] >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> ... proceed with SwitchOnSmi... >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Thank you! >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>> v8-dev mailing list >>>>>>>>>>>>>>>>> [email protected] >>>>>>>>>>>>>>>>> http://groups.google.com/group/v8-dev >>>>>>>>>>>>>>>>> --- >>>>>>>>>>>>>>>>> You received this message because you are subscribed to >>>>>>>>>>>>>>>>> the Google Groups "v8-dev" group. >>>>>>>>>>>>>>>>> To unsubscribe from this group and stop receiving emails >>>>>>>>>>>>>>>>> from it, send an email to [email protected]. >>>>>>>>>>>>>>>>> To view this discussion on the web visit >>>>>>>>>>>>>>>>> https://groups.google.com/d/msgid/v8-dev/6df36377-1d02-4de9-aec4-5890003af416n%40googlegroups.com >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> <https://groups.google.com/d/msgid/v8-dev/6df36377-1d02-4de9-aec4-5890003af416n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>>>>>>>>>> . >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>> -- >>>>>>>>>>>>>> v8-dev mailing list >>>>>>>>>>>>>> [email protected] >>>>>>>>>>>>>> http://groups.google.com/group/v8-dev >>>>>>>>>>>>>> --- >>>>>>>>>>>>>> You received this message because you are subscribed to the >>>>>>>>>>>>>> Google Groups "v8-dev" group. >>>>>>>>>>>>>> To unsubscribe from this group and stop receiving emails from >>>>>>>>>>>>>> it, send an email to [email protected]. >>>>>>>>>>>>>> >>>>>>>>>>>>> To view this discussion on the web visit >>>>>>>>>>>>>> https://groups.google.com/d/msgid/v8-dev/fd87fb54-8831-4dea-aa22-5045bd892e61n%40googlegroups.com >>>>>>>>>>>>>> >>>>>>>>>>>>>> <https://groups.google.com/d/msgid/v8-dev/fd87fb54-8831-4dea-aa22-5045bd892e61n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>>>>>>> . >>>>>>>>>>>>>> >>>>>>>>>>>>> -- >>>>>>>>>>>> -- >>>>>>>>>>>> v8-dev mailing list >>>>>>>>>>>> [email protected] >>>>>>>>>>>> http://groups.google.com/group/v8-dev >>>>>>>>>>>> --- >>>>>>>>>>>> You received this message because you are subscribed to the >>>>>>>>>>>> Google Groups "v8-dev" group. >>>>>>>>>>>> To unsubscribe from this group and stop receiving emails from >>>>>>>>>>>> it, send an email to [email protected]. >>>>>>>>>>>> >>>>>>>>>>> To view this discussion on the web visit >>>>>>>>>>>> https://groups.google.com/d/msgid/v8-dev/064ef28d-0577-4541-a94f-8c0332eb0d86n%40googlegroups.com >>>>>>>>>>>> >>>>>>>>>>>> <https://groups.google.com/d/msgid/v8-dev/064ef28d-0577-4541-a94f-8c0332eb0d86n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>>>>> . >>>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>> -- >>>>>>>>>> v8-dev mailing list >>>>>>>>>> [email protected] >>>>>>>>>> http://groups.google.com/group/v8-dev >>>>>>>>>> --- >>>>>>>>>> You received this message because you are subscribed to the >>>>>>>>>> Google Groups "v8-dev" group. >>>>>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>>>>> send an email to [email protected]. >>>>>>>>>> >>>>>>>>> To view this discussion on the web visit >>>>>>>>>> https://groups.google.com/d/msgid/v8-dev/ac4a9ade-2889-4548-bb50-d2fa723e580an%40googlegroups.com >>>>>>>>>> >>>>>>>>>> <https://groups.google.com/d/msgid/v8-dev/ac4a9ade-2889-4548-bb50-d2fa723e580an%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>>> . >>>>>>>>>> >>>>>>>>> -- >>>>>>> -- >>>>>>> v8-dev mailing list >>>>>>> [email protected] >>>>>>> http://groups.google.com/group/v8-dev >>>>>>> --- >>>>>>> You received this message because you are subscribed to the Google >>>>>>> Groups "v8-dev" group. >>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>> send an email to [email protected]. >>>>>>> >>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/d/msgid/v8-dev/0bbac741-4b9b-4c66-aa69-922cc8e99e49n%40googlegroups.com >>>>>>> >>>>>>> <https://groups.google.com/d/msgid/v8-dev/0bbac741-4b9b-4c66-aa69-922cc8e99e49n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>> . >>>>>>> >>>>>> -- >>>>> -- >>>>> v8-dev mailing list >>>>> [email protected] >>>>> http://groups.google.com/group/v8-dev >>>>> --- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "v8-dev" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> >>>> To view this discussion on the web visit >>>>> https://groups.google.com/d/msgid/v8-dev/67731bdf-0451-4006-9b90-36cac7886c50n%40googlegroups.com >>>>> >>>>> <https://groups.google.com/d/msgid/v8-dev/67731bdf-0451-4006-9b90-36cac7886c50n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> -- >> -- >> v8-dev mailing list >> [email protected] >> http://groups.google.com/group/v8-dev >> --- >> You received this message because you are subscribed to the Google Groups >> "v8-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> > To view this discussion on the web visit >> https://groups.google.com/d/msgid/v8-dev/a028be28-fdbe-4a99-9245-3a9566464f7fn%40googlegroups.com >> >> <https://groups.google.com/d/msgid/v8-dev/a028be28-fdbe-4a99-9245-3a9566464f7fn%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> > -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/23fde598-4d3a-4af2-8fe1-b1d438671d35n%40googlegroups.com.
