...
<rdar://problem/65272138>
Created attachment 404199 [details] patch
Created attachment 404200 [details] patch
Comment on attachment 404200 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=404200&action=review r=me with nit. > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:14743 > + jit.store32(CCallHelpers::TrustedImm32(DoesGCCheck::encode(true, DoesGCCheck::Special::Uninitialized)), CCallHelpers::Address(GPRInfo::regT0)); Nit: I think this can be DoesGCCheck(). But maybe I'm missing something.
Comment on attachment 404200 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=404200&action=review >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:14743 >> + jit.store32(CCallHelpers::TrustedImm32(DoesGCCheck::encode(true, DoesGCCheck::Special::Uninitialized)), CCallHelpers::Address(GPRInfo::regT0)); > > Nit: I think this can be DoesGCCheck(). But maybe I'm missing something. we don't have an accessor to read out the bits as is, that's why I'm calling encode
Created attachment 404202 [details] patch for landing
Created attachment 404204 [details] patch for landing
Committed r264330: <https://trac.webkit.org/changeset/264330> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404204 [details].
Hi, this causes JSC stress tests to run forever on x86_64 when cloop is enabled. I think we need a different skip condition since --returnEarlyFromInfiniteLoopsForFuzzing is not implemented for cloop, right? Is there a way to skip tests depending on whether JIT is enabled? Concerning: the tests are still passing an aarch64, meaning this test must be skipped despite the skip if $architecture != "arm64". That indicates we probably aren't detecting aarch64 correctly.
(In reply to Michael Catanzaro from comment #9) > Hi, this causes JSC stress tests to run forever on x86_64 when cloop is > enabled. I think we need a different skip condition since > --returnEarlyFromInfiniteLoopsForFuzzing is not implemented for cloop, > right? Is there a way to skip tests depending on whether JIT is enabled? > > Concerning: the tests are still passing an aarch64, meaning this test must > be skipped despite the skip if $architecture != "arm64". That indicates we > probably aren't detecting aarch64 correctly. I can fix tomorrow or Monday. We probably should add $vm.isCloop() if it doesn’t exist already, and then return early from the test. Is the ARM64 bot cloop? Does it invoke run-JSC-stress-tests with “—arch arm64”?
(In reply to Saam Barati from comment #10) > I can fix tomorrow or Monday. We probably should add $vm.isCloop() if it > doesn’t exist already, and then return early from the test. Thanks. See: bug #214491. I forgot to leave a link to that he re. > Is the ARM64 bot cloop? Yes. (Our internal testers actually only test cloop. The public bots are good enough to keep the JIT working. :) > Does it invoke run-JSC-stress-tests with “—arch arm64”? No, none of our runners use the --arch option. (Didn't know about that option.) It's determined by determineArchitectureFromELFBinary() in run-jsc-stress-tests, which I touched last in r261862. I might have to try to get access to a binary file to see if that function is working properly.
(In reply to Michael Catanzaro from comment #11) > (In reply to Saam Barati from comment #10) > > I can fix tomorrow or Monday. We probably should add $vm.isCloop() if it > > doesn’t exist already, and then return early from the test. > > Thanks. See: bug #214491. I forgot to leave a link to that he re. > > > Is the ARM64 bot cloop? > > Yes. (Our internal testers actually only test cloop. The public bots are > good enough to keep the JIT working. :) > > > Does it invoke run-JSC-stress-tests with “—arch arm64”? > > No, none of our runners use the --arch option. (Didn't know about that > option.) It's determined by determineArchitectureFromELFBinary() in > run-jsc-stress-tests, which I touched last in r261862. I might have to try > to get access to a binary file to see if that function is working properly. Yeah, if you're not passing "--arch", it's pretty likely that's the issue. I've ensured many times that these checks work in the tests themselves (because I've added many of them).