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.
