Bug 1039993 modified Float32ToDouble on the ARM and MIPS to not useAtStart its argument due to LSRA issues, however it can still be optimized to useAtStart its argument when not using the LSRA and this will help asm.js code which often has a lot of float32 to double conversions.
We run into the LSRA issues from bug 1039993, so the code dynamically avoids useAtStart for the when using the LSRA. Updated the debug checks added in bug 1039993 to only apply when using the LSRA. Bug 1039993 appears to have dropped a necessary check which has been restored: JS_ASSERT(!(hasUseRegister && hasUseRegisterAtStart));
Attachment #8495231 - Flags: review?(mrosenberg)
Comment on attachment 8495231 [details] [diff] [review] Optimize Float32ToDouble to useAtStart its argument. Review of attachment 8495231 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/LiveRangeAllocator.cpp @@ +805,2 @@ > > +# if defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS) This should be unnecessary, and possibly wrong. hasUnaliasedDouble is only true on ARM, and only some ARM chips at that. The hasMultiAlias should also protect the rest of the cases as well. ::: js/src/jit/Lowering.cpp @@ +1784,5 @@ > // should still be, however, there is a bug in LSRA's implementation of > // *AtStart, which is quite fundamental. This should be reverted when that > // is fixed, or lsra is deprecated. > + if (gen->optimizationInfo().registerAllocator() == RegisterAllocator_LSRA) > + lir = new(alloc()) LFloat32ToDouble(useRegister(opd)); nit: new (alloc()), here and below.
Attachment #8495231 - Flags: review?(mrosenberg) → review+
Thank you for the quick review. Re-enabled the assertions on all backends, thanks. Carrying forward r+. Try run: https://tbpl.mozilla.org/?tree=Try&rev=b89f0ca2f6b0
Some orange in the try run in comment 3, but it does not appear related to this patch. Here's another try build that included this patch and while it has some different orange and red it does not appear related: https://tbpl.mozilla.org/?tree=Try&rev=eb54d95aacba
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.