Fixing bug with shared bailouts

[rajatd]

Shared bailouts for InlineMathFloor/Ceil/Round were bypassing the code on the bailout path that sets up data specific to a bailout (bailout kind, polymorphic inline cache index etc.) on the bailout record and were jumping directly to the bailout label. Fixed that.

---

This change is

[rajatd]

@pleath @MikeHolman @LouisLaf

[pleath]

I wonder if we're asking for trouble unlinking a label that the caller already put in the IR stream (presumably for reasons of its own). Would it work to assert that the label is already unlinked? Is there a caller that requires unlinking now? If so, I wonder if the caller shouldn't unlink it itself.

[rajatd]

Hmm.. The lowering functions for InlineMathFloor et.al. require unlinking now. I could, however, not insert the label in those routines and just pass it to GenerateBailout as collectRuntimeStatsLabel and insert it before the current instruction there.

[pleath]

That sounds better to me. The logic in the caller shouldn’t require/expect the label to remain where it was put in the IR stream.

--Paul

From: Rajat Dua [mailto:notifications@github.com]
Sent: Monday, May 9, 2016 5:20 PM
To: Microsoft/ChakraCore ChakraCore@noreply.github.com
Cc: Paul Leathers pleath@microsoft.com; Mention mention@noreply.github.com
Subject: Re: [Microsoft/ChakraCore] Fixing bug with shared bailouts (#958)

In lib/Backend/Lower.cpphttps://github.com//pull/958#discussion_r62595416:

@@ -12174,14 +12173,21 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L

     return bailOutLabel;

 }

• // Add helper label to trigger layout.
• if (collectRuntimeStatsLabel)
• {

•    collectRuntimeStatsLabel->Unlink();
  

Hmm.. The lowering functions for InlineMathFloor et.al. require unlinking now. I could, however, not insert the label in those routines and just pass it to GenerateBailout as collectRuntimeStatsLabel and insert it before the current instruction there.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHubhttps://github.com//pull/958/files/a596859bd23086a43a5027cab91cc8696ac6d331#r62595416

[pleath]

LGTM.

[rajatd]

Thanks, merging.