Bug 151841

Summary: Add an option to emit instructions validating exceptions in the DFG rather than always emiting them.
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Keith Miller
Reported 2015-12-03 17:42:54 PST
Add an option to emit instructions validating exceptions in the DFG rather than always emiting them.
Attachments
Patch (2.18 KB, patch)
2015-12-03 17:46 PST, Keith Miller 		no flags
DetailsFormatted DiffDiff
Patch (5.05 KB, patch)
2015-12-03 19:33 PST, Keith Miller 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2015-12-03 17:46:25 PST
Created attachment 266582 [details] Patch
Michael Saboff
Comment 2 2015-12-03 18:07:13 PST
Comment on attachment 266582 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266582&action=review I see that Saam added these JIT asserts to help find what operations emit exception checks. Given that you have the desire to turn them off by default, it seems to me that it makes more sense to eliminated them altogether. Want to hear what others (Saam) think. > Source/JavaScriptCore/runtime/Options.h:329 > + v(bool, validateExceptionHandlingInDFG, false, nullptr) \ Add a description string for the option.
Keith Miller
Comment 3 2015-12-03 18:32:04 PST
> I see that Saam added these JIT asserts to help find what operations emit > exception checks. Given that you have the desire to turn them off by > default, it seems to me that it makes more sense to eliminated them > altogether. Want to hear what others (Saam) think. I spoke with Saam offline and he mirrored your opinion. Instead, I'll have the option on by default in debug builds. > > Source/JavaScriptCore/runtime/Options.h:329 > > + v(bool, validateExceptionHandlingInDFG, false, nullptr) \ > > Add a description string for the option. Fixed.
Keith Miller
Comment 4 2015-12-03 19:33:31 PST
Created attachment 266589 [details] Patch
Saam Barati
Comment 5 2015-12-03 21:23:40 PST
Comment on attachment 266589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266589&action=review r=me > Source/JavaScriptCore/jit/AssemblyHelpers.h:1005 > + void jitReleaseAssertNoException(); We could also call this crashIfVMHasException or something like that.
Saam Barati
Comment 6 2015-12-04 00:44:18 PST
On second thought, I do like the releaseAssert name
WebKit Commit Bot
Comment 7 2015-12-04 10:45:59 PST
Comment on attachment 266589 [details] Patch Clearing flags on attachment: 266589 Committed r193424: <http://trac.webkit.org/changeset/193424>
WebKit Commit Bot
Comment 8 2015-12-04 10:46:02 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.