A quick update from an offline conversation with Andreas: It looks like we already clear the upper half of i32 register params in the function prologues almost everywhere, so the unsigned conversion in the generic js-to-wasm wrapper does look redundant. The only case that is missing is on no-sandbox builds, specifically in Liftoff, because so far this was only done for sandbox-related reasons. It seems fine to extend that to all configs, that would be more consistent and would allow us to keep the torque code platform-independent.
On Thu, Aug 8, 2024 at 10:20 AM 'Andreas Haas' via v8-dev < [email protected]> wrote: > No, the assembly builtin loads the value as it is from the buffer on the > stack into the registers, it does not know if it is loading a 32-bit value > or a 64-bit value. > > On Thu, Aug 8, 2024 at 10:17 AM Zhao Jiazhong <[email protected]> > wrote: > >> I still think loong64 port needs to modify the torque builtin. >> I ported JSPI to loong64 according to arm64 port, and it seems the assembly >> builtin >> <https://source.chromium.org/chromium/chromium/src/+/refs/tags/119.0.6045.214:v8/src/builtins/arm64/builtins-arm64.cc;l=4158> >> just load the parameters by full 64 bits and pass them to JIT code? I >> wonder if the builtin have the information about the bit width of the >> arguments. >> >> I uploaded my patch to gerrit: [loong64][wasm][jspi] Initial port of >> JSPI to loongarch64 architecture >> <https://chromium-review.googlesource.com/c/v8/v8/+/5772152>, welcome to >> review. Thanks! >> On Wednesday, August 7, 2024 at 7:08:49 PM UTC+8 Zhao Jiazhong wrote: >> >>> Okay, I will have a look at the assembly builtin. Thanks for your >>> detailed information! >>> >>> On Wednesday, August 7, 2024 at 6:35:25 PM UTC+8 [email protected] wrote: >>> >>>> The code there converts an incoming Smi parameter to an int32 and >>>> writes that int32 into a buffer on the stack. This buffer is then read in >>>> an assembly builtin [1], where the int32 is loaded either into a register >>>> or pushed on the stack. I think you actually have a bug in the assembly >>>> builtin, not in the Torque builtin. >>>> >>>> There is a fuzzer-like mjsunit test [2] that allows you to test this >>>> code very well. If you set the `debug` variable at the beginning of the >>>> test to true, you enable the output and increase the number of test cases. >>>> It's very helpful to debug your implementation. >>>> >>>> Cheers, Andreas >>>> >>>> [1] >>>> https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/loong64/builtins-loong64.cc;l=3037;drc=9b2c99f8734501549f5726bc187d3f05dfaa7af0 >>>> >>>> [2] >>>> https://source.chromium.org/chromium/chromium/src/+/main:v8/test/mjsunit/wasm/torque-wrapper.js;l=25;drc=b2df50672c06c070ecb137570d1072a3835ac4ca >>>> >>>> On Wed, Aug 7, 2024 at 12:02 PM Zhao Jiazhong <[email protected]> >>>> wrote: >>>> >>>>> Losts of tests failed on loong64 *with* the `Unsigned`, and they >>>>> would pass if I remove the `Unsigned` for loong64. >>>>> >>>>> It's because LoongArch64, MIPS64 and perhaps RISCV64 all generally >>>>> need to sign-extend 32-bit values in 64-bit registers according to their >>>>> calling convention. But the convert to unsigned then to intptr will >>>>> generate a zero-extended 32-bit value, which caused the failue on loong64 >>>>> port. Not sure why x64 needs the `Unsigned`. >>>>> On Wednesday, August 7, 2024 at 5:51:42 PM UTC+8 [email protected] >>>>> wrote: >>>>> >>>>>> But will these tests not also fail on loong64 then? Why do you expect >>>>>> different behavior there? >>>>>> >>>>>> On Wed, Aug 7, 2024 at 11:44 AM Zhao Jiazhong <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Some tests like `wasm-spec-tests/address` failed on x64 if remove >>>>>>> the `Unsigned`. >>>>>>> >>>>>>> On Wednesday, August 7, 2024 at 5:29:32 PM UTC+8 [email protected] >>>>>>> wrote: >>>>>>> >>>>>>>> I think it should not be a problem to remove the `Unsigned` there >>>>>>>> for other platforms as well, if it works. What's happening there is >>>>>>>> that a >>>>>>>> Smi gets converted into an int32 and then stored in a 64-bit slot on >>>>>>>> the >>>>>>>> stack. If the second half of the 64-bit slot gets filled with ones or >>>>>>>> with >>>>>>>> zeroes does not matter, I think. If it matters, then there are tests in >>>>>>>> place. >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Aug 7, 2024 at 11:06 AM Zhao Jiazhong < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> It's a good idea, I will use this method. Thanks! >>>>>>>>> >>>>>>>>> On Wednesday, August 7, 2024 at 5:00:53 PM UTC+8 >>>>>>>>> [email protected] wrote: >>>>>>>>> >>>>>>>>>> I would give this "condition" a name >>>>>>>>>> `@if(NEEDS_SPECIAL_INT32_TO_INT64_SIGN_EXTENSION)` >>>>>>>>>> (maybe find something more descriptive) and then set this build >>>>>>>>>> flag for all architectures that need that. >>>>>>>>>> >>>>>>>>>> On Wed, Aug 7, 2024 at 10:52 AM Zhao Jiazhong < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Nico, >>>>>>>>>>> >>>>>>>>>>> Thanks for your information, it indeed works! >>>>>>>>>>> But it seems that the @if and @ifnot annotations can't handle >>>>>>>>>>> two conditions at once like `@if(V8_TARGET_ARCH_LOONG64 || >>>>>>>>>>> V8_TARGET_ARCH_MIPS64)`, and I didn't find something like `@elif`. >>>>>>>>>>> I thinks MIPS64 is likely need this change too, but add >>>>>>>>>>> @if(MIPS64) and @ifnot(MIPS64) in @ifnot(LOONG64) seems ugly. Do >>>>>>>>>>> you have >>>>>>>>>>> any suggestions? Thank you very much! >>>>>>>>>>> >>>>>>>>>>> Yours, >>>>>>>>>>> Zhao Jiazhong >>>>>>>>>>> On Wednesday, August 7, 2024 at 2:21:24 PM UTC+8 >>>>>>>>>>> [email protected] wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Zhao, >>>>>>>>>>>> >>>>>>>>>>>> you can use Torque's @if and @ifnot annotations to make such >>>>>>>>>>>> distinctions (check @if(TAGGED_SIZE_8_BYTES) for an example). You >>>>>>>>>>>> then need >>>>>>>>>>>> to set this from the C++ side in torque-parser.cc >>>>>>>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:v8/src/torque/torque-parser.cc;l=46;drc=35bb990bac45ef15807c9eab4d60b8078b65f038> >>>>>>>>>>>> and >>>>>>>>>>>> for that you can use the usual `V8_TARGET_ARCH_XXX`. The places >>>>>>>>>>>> where you >>>>>>>>>>>> can use such annotations are a bit restricted if I remember >>>>>>>>>>>> correctly, but >>>>>>>>>>>> it should be enough to support your case (maybe see this >>>>>>>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/promise-misc.tq;l=82;drc=7857eb34db42f339b337c6bdfb0d10deb14862f3> >>>>>>>>>>>> for an example). Hope that helps. >>>>>>>>>>>> >>>>>>>>>>>> Cheers >>>>>>>>>>>> Nico >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Wed, Aug 7, 2024 at 4:18 AM Zhao Jiazhong < >>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi all, >>>>>>>>>>>>> >>>>>>>>>>>>> I'm porting JSPI to loong64 port, and find an issue that in >>>>>>>>>>>>> JSToWasmWrapperHelper >>>>>>>>>>>>> <https://source.chromium.org/chromium/chromium/src/+/refs/tags/119.0.6045.214:v8/src/builtins/js-to-wasm.tq;l=559>, >>>>>>>>>>>>> a 32-bit value is converted to unsigned then converted to intptr, >>>>>>>>>>>>> which >>>>>>>>>>>>> leading to a zero-extended value, but on LoongArch64, we need the >>>>>>>>>>>>> 32-bit >>>>>>>>>>>>> value to be sign-extended in 64-bit registers. >>>>>>>>>>>>> >>>>>>>>>>>>> I don't want to change the behavior on other arches, but the >>>>>>>>>>>>> builtin is written in torque, I suppose I can't use >>>>>>>>>>>>> `V8_TARGET_ARCH_XXX` >>>>>>>>>>>>> macro in it, so is there a way to distinguish v8 target arch >>>>>>>>>>>>> in torque builtins? Thanks! >>>>>>>>>>>>> >>>>>>>>>>>>> Yours, >>>>>>>>>>>>> Zhao Jiazhong >>>>>>>>>>>>> >>>>>>>>>>>>> -- >>>>>>>>>>>>> -- >>>>>>>>>>>>> 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/c2790cc7-513d-4296-8531-f620fe93e038n%40googlegroups.com >>>>>>>>>>>>> <https://groups.google.com/d/msgid/v8-dev/c2790cc7-513d-4296-8531-f620fe93e038n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>>>>>> . >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> Nico Hartmann | Software Engineer | [email protected] | Chrome >>>>>>>>>>>> - V8 >>>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> -- >>>>>>>>>>> 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/88402770-d013-4847-8688-cfdf6cf0793an%40googlegroups.com >>>>>>>>>>> <https://groups.google.com/d/msgid/v8-dev/88402770-d013-4847-8688-cfdf6cf0793an%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>>>> . >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Nico Hartmann | Software Engineer | [email protected] | Chrome >>>>>>>>>> - V8 >>>>>>>>>> >>>>>>>>> -- >>>>>>>>> -- >>>>>>>>> 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/3e4f56ab-a3ab-4e12-941b-9319c2e29a68n%40googlegroups.com >>>>>>>>> <https://groups.google.com/d/msgid/v8-dev/3e4f56ab-a3ab-4e12-941b-9319c2e29a68n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>>>> . >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> >>>>>>>> Andreas Haas >>>>>>>> >>>>>>>> Software Engineer >>>>>>>> >>>>>>>> [email protected] >>>>>>>> >>>>>>>> >>>>>>>> Google Germany GmbH >>>>>>>> >>>>>>>> Erika-Mann-Straße 33 >>>>>>>> >>>>>>>> 80636 München >>>>>>>> >>>>>>>> >>>>>>>> Geschäftsführer: Paul Manicle, Liana Sebastian >>>>>>>> >>>>>>>> Registergericht und -nummer: Hamburg, HRB 86891 >>>>>>>> >>>>>>>> Sitz der Gesellschaft: Hamburg >>>>>>>> >>>>>>>> >>>>>>>> Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise >>>>>>>> erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes >>>>>>>> weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich >>>>>>>> bitte >>>>>>>> wissen, dass die E-Mail an die falsche Person gesendet wurde. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> This e-mail is confidential. If you received this communication by >>>>>>>> mistake, please don't forward it to anyone else, please erase all >>>>>>>> copies >>>>>>>> and attachments, and please let me know that it has gone to the wrong >>>>>>>> person. >>>>>>>> >>>>>>>> -- >>>>>>> -- >>>>>>> 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/06ce8d10-ee9a-4993-946e-1c6bc3e352cdn%40googlegroups.com >>>>>>> <https://groups.google.com/d/msgid/v8-dev/06ce8d10-ee9a-4993-946e-1c6bc3e352cdn%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>>> . >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> >>>>>> Andreas Haas >>>>>> >>>>>> Software Engineer >>>>>> >>>>>> [email protected] >>>>>> >>>>>> >>>>>> Google Germany GmbH >>>>>> >>>>>> Erika-Mann-Straße 33 >>>>>> >>>>>> 80636 München >>>>>> >>>>>> >>>>>> Geschäftsführer: Paul Manicle, Liana Sebastian >>>>>> >>>>>> Registergericht und -nummer: Hamburg, HRB 86891 >>>>>> >>>>>> Sitz der Gesellschaft: Hamburg >>>>>> >>>>>> >>>>>> Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise >>>>>> erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes >>>>>> weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich >>>>>> bitte >>>>>> wissen, dass die E-Mail an die falsche Person gesendet wurde. >>>>>> >>>>>> >>>>>> >>>>>> This e-mail is confidential. If you received this communication by >>>>>> mistake, please don't forward it to anyone else, please erase all copies >>>>>> and attachments, and please let me know that it has gone to the wrong >>>>>> person. >>>>>> >>>>>> -- >>>>> -- >>>>> 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/6d148e25-963e-4c60-b7b5-5f7c11c64e2fn%40googlegroups.com >>>>> <https://groups.google.com/d/msgid/v8-dev/6d148e25-963e-4c60-b7b5-5f7c11c64e2fn%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> >>>> >>>> -- >>>> >>>> Andreas Haas >>>> >>>> Software Engineer >>>> >>>> [email protected] >>>> >>>> >>>> Google Germany GmbH >>>> >>>> Erika-Mann-Straße 33 >>>> >>>> 80636 München >>>> >>>> >>>> Geschäftsführer: Paul Manicle, Liana Sebastian >>>> >>>> Registergericht und -nummer: Hamburg, HRB 86891 >>>> >>>> Sitz der Gesellschaft: Hamburg >>>> >>>> >>>> Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise >>>> erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes >>>> weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte >>>> wissen, dass die E-Mail an die falsche Person gesendet wurde. >>>> >>>> >>>> >>>> This e-mail is confidential. If you received this communication by >>>> mistake, please don't forward it to anyone else, please erase all copies >>>> and attachments, and please let me know that it has gone to the wrong >>>> person. >>>> >>>> -- >> -- >> 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/7b281bc6-f010-4e06-9f10-60f74163e010n%40googlegroups.com >> <https://groups.google.com/d/msgid/v8-dev/7b281bc6-f010-4e06-9f10-60f74163e010n%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> > > > -- > > Andreas Haas > > Software Engineer > > [email protected] > > > Google Germany GmbH > > Erika-Mann-Straße 33 > > 80636 München > > > Geschäftsführer: Paul Manicle, Liana Sebastian > > Registergericht und -nummer: Hamburg, HRB 86891 > > Sitz der Gesellschaft: Hamburg > > > Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten > haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, > löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, > dass die E-Mail an die falsche Person gesendet wurde. > > > > This e-mail is confidential. If you received this communication by > mistake, please don't forward it to anyone else, please erase all copies > and attachments, and please let me know that it has gone to the wrong > person. > > -- > -- > 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/CAELSTvfHEeOkJ2kotTyrJk9djm48nRUYLnWHCn1jipd0ZVHyJg%40mail.gmail.com > <https://groups.google.com/d/msgid/v8-dev/CAELSTvfHEeOkJ2kotTyrJk9djm48nRUYLnWHCn1jipd0ZVHyJg%40mail.gmail.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/CALowwNZ_ZdkAUt-aejGLMiEisg2jDxyyVta4oY4S1SLshkZ9Eg%40mail.gmail.com.
