Hi, looking at the implementations for signed and unsigned 32-bit divisions in MachineOperatorReducer, it seems like there's some code that can be factored together. Do you think it's worth doing it since I'm introducing the 64-bit versions of these?
Also, partially related, I saw that the handling of kInt32Mul is slightly different from the one of kInt32MulWithOverflow: in particular, it looks like the latter only checks for multiplications by -1 or 2 (which is transformed into an addition) here <https://github.com/v8/v8/blob/7a4a6cc6a85650ee91344d0dbd2c53a8fa8dce04/src/compiler/machine-operator-reducer.cc#L391> and 0 and 1 are handled separately by ReduceProjection here <https://github.com/v8/v8/blob/7a4a6cc6a85650ee91344d0dbd2c53a8fa8dce04/src/compiler/machine-operator-reducer.cc#L1311>, while the former handles 0,1,-1, power of twos and consecutive multiplications by a constant. Could the same set of optimizations be applied for both? Hope I'm not being too picky here. Have a nice one, Alvise On Wednesday, June 29, 2022 at 1:36:07 PM UTC+2 Andreas Haas wrote: > Hi, > > The reason could be that this code was written before the addition of > BigInt to JavaScript, and at that time these reductions were not needed > because 64-bit divisions were not used. I could imagine that when support > for WebAssembly and BigInt was added, nobody remembered to add these > reductions. > > I would be happy to review your changes. > > Cheers, Andreas > > On Tue, Jun 28, 2022 at 2:26 PM Alvise de Faveri Tron <[email protected]> > wrote: > >> Hi all! >> >> I was looking at MachineOperatorReducer in src/compiler and I've noticed >> that, while there are reductions for signed and unsigned 32-bit integer >> divisions (e.g. >> MachineOperatorReducer::ReduceInt32Div), there is no such thing for >> 64-bit divisions. >> >> Is there a reason why 64-bit divisions are not optimized? Maybe the *ctz >> *operation is too slow on 64 bits if there is no intrinsic *ctz* >> instruction? That's just a wild (and likely incorrect) guess, I'd just be >> curious to know more. >> >> To give some context, I am part of the ongoing effort to port V8 to >> RISCV32, and we noticed this possibility for optimization in this issue >> <https://github.com/riscv-collab/v8/issues/574>. >> >> If there is no specific reason, I'd be happy to make this contribution. >> >> Best to all, >> - Alvise >> >> -- >> -- >> 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/8f6d3e85-0141-416c-a82a-ed622b2ff30bn%40googlegroups.com >> >> <https://groups.google.com/d/msgid/v8-dev/8f6d3e85-0141-416c-a82a-ed622b2ff30bn%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/74af643d-0b83-467c-b644-85adb789f7efn%40googlegroups.com.
