Closed Bug 990022 Opened 9 years ago Closed 9 years ago

BaselineCompiler: Refactor profiler instrumentation code in BaselineIC.cpp

Categories

(Core :: JavaScript Engine: JIT, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: djvj, Assigned: djvj)

Details

Attachments

(1 file)

Lots of really redundant code in BaselineIC where we have very similar code for emitting profielr instrumentation before calls.  This can and should be refactored and shortened easily.
Drops about 150 lines from BaselineIC.cpp
Attachment #8399423 - Flags: review?(efaustbmo)
Comment on attachment 8399423 [details] [diff] [review]
refactor-baseline-ic-profiling-instrumentation.patch

Review of attachment 8399423 [details] [diff] [review]:
-----------------------------------------------------------------

This was a much needed cleanup. Thanks for doing this! r=me

::: js/src/jit/BaselineIC.cpp
@@ +724,5 @@
> +void
> +ICStubCompiler::emitProfilingUpdate(MacroAssembler &masm, GeneralRegisterSet regs,
> +                                    uint32_t stubPcOffset)
> +{
> +    emitProfilingUpdate(masm, regs.takeAny(), regs.takeAny(), stubPcOffset);

We want to re-add the registers to |regs| here, right? It doesn't look like all the callsites create a new register set to call this function.
Attachment #8399423 - Flags: review?(efaustbmo) → review+
(In reply to Eric Faust [:efaust] from comment #2)
> We want to re-add the registers to |regs| here, right? It doesn't look like
> all the callsites create a new register set to call this function.

Well, it's passed in by-value, so it's a copy of the register set.  It doesn't affect the caller's copy.
https://hg.mozilla.org/mozilla-central/rev/0228344cc188
Assignee: nobody → kvijayan
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
The following changeset is now in Firefox Nightly:

> 0228344cc188 Bug 990022 - Refactor BaselineIC profiler pseudo-stack update code. r=efaust

Nightly Build Information:

        ID: 20140402030201
 Changeset: 4941a2ac0786109b08856738019b016a6c5a66a6
   Version: 31.0a1
      TBPL: https://tbpl.mozilla.org/?rev=4941a2ac0786
       URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central

Download Links:

>         Linux x86: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-i686.tar.bz2
>      Linux x86_64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64.tar.bz2
> Linux x86_64 ASAN: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64-asan.tar.bz2
>               Mac: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.mac.dmg
>             Win32: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win32.installer.exe
>             Win64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win64-x86_64.installer.exe

Previous Nightly Build Information:

        ID: 20140401030203
 Changeset: 1417d180a1d8665b1a91b897d1cc4cc31e7980d4
   Version: 31.0a1
      TBPL: https://tbpl.mozilla.org/?rev=1417d180a1d8
       URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-01-03-02-03-mozilla-central
You need to log in before you can comment on or make changes to this bug.