115684 2013-05-06 17:06:06 -0700 ftp branch: Fix broken 32-bit build + clean up JITStubs.cpp a bit. 2013-05-06 22:06:32 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mark.lam mark.lam fpizlo ggaren mhahnenberg msaboff oliver oldest_to_newest 885515 0 mark.lam 2013-05-06 17:06:06 -0700 Remedy minor bit rotting in DFGSpeculativeJIT32_64.cpp. JITStubs.cpp has a case for CPU(X86_64) && USE(JSVALUE32_64) which doesn't make sense. Will delete this block of unnecessary code. Also will add some line breaks and update some comments to more clearly delineate between the sections of code in JITStubs.cpp. 885516 1 200857 mark.lam 2013-05-06 17:10:11 -0700 Created attachment 200857 the patch. 885517 2 200857 ggaren 2013-05-06 17:14:45 -0700 Comment on attachment 200857 the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=200857&action=review > Source/JavaScriptCore/jit/JITStubs.cpp:312 > -#else // USE(JSVALUE32_64) > +#else // !USE(JSVALUE32_64) > > #if COMPILER(GCC) && CPU(X86_64) > I believe the style in other WebKit code is to annotate a dependent preprocessor macro with the heading for the macro's start. So, in this case, "USE(JSVALUE32_64)" is correct. 885526 3 mark.lam 2013-05-06 17:42:18 -0700 (In reply to comment #2) > (From update of attachment 200857 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=200857&action=review > > > Source/JavaScriptCore/jit/JITStubs.cpp:312 > > -#else // USE(JSVALUE32_64) > > +#else // !USE(JSVALUE32_64) > > > > #if COMPILER(GCC) && CPU(X86_64) > > > > I believe the style in other WebKit code is to annotate a dependent preprocessor macro with the heading for the macro's start. So, in this case, "USE(JSVALUE32_64)" is correct. I'll revert this !USE(JSVALUE32_64) change for this commit, but for future reference, what is the right thing to do then when we have a case like the following? #if CPU(x86) ... #elif CPU(X86_64) ... #else // what comment is appropriate here? #endif // whacomment is appropriate here? 885533 4 mark.lam 2013-05-06 17:47:01 -0700 Thanks for the review. Landed in r149655: <http://trac.webkit.org/changeset/149655>. 885601 5 ggaren 2013-05-06 22:06:32 -0700 > what is the right thing to do then when we have a case like the following? > > #if CPU(x86) > ... > #elif CPU(X86_64) > ... > #else // what comment is appropriate here? > #endif // whacomment is appropriate here? I don't think we have a clear guideline on this. Thinking about it now, I might choose not to use #elif, and instead: #if CPU(X86) #endif // CPU(X86) #if CPU(X86_64) #endif // CPU(X86_64) #if !CPU(X86) && !CPU(X86_64) #endif // !CPU(X86) && !CPU(X86_64) The phrase "else" is hard to decipher when it flows hundred of lines after an "if", possibly with intervening "#elif". 200857 2013-05-06 17:10:11 -0700 2013-05-06 17:14:44 -0700 the patch. bug-115684.patch text/plain 5248 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTQ5NjUwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBA CisyMDEzLTA1LTA2ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBG aXggYnJva2VuIDMyLWJpdCBidWlsZCArIHNvbWUgY2xlYW4gdXAgaW4gSklUU3R1YnMuY3BwLgor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTE1Njg0Lgor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogZGZnL0RG R1NwZWN1bGF0aXZlSklUMzJfNjQuY3BwOgorICAgICAgICAoSlNDOjpERkc6OlNwZWN1bGF0aXZl SklUOjpjb21waWxlKToKKyAgICAgICAgKiBqaXQvSklUU3R1YnMuY3BwOgorICAgICAgICAtIHJl bW92ZWQgdW5uZWVkZWQgc3R1YnMgZm9yIENQVShYODZfNjQpICYmIFVTRShKU1ZBTFVFMzJfNjQp LgorICAgICAgICAtIGFkZGVkIHNvbWUgbGluZSBicmVha3MgYW5kIHVwZGF0ZWQgY29tbWVudHMg dG8gbW9yZSBjbGVhcmx5IGRlbGluZWF0ZQorICAgICAgICAgIGJldHdlZW4gcG9ydHMvY29uZmln dXJhdGlvbnMgb2Ygc3R1YiBjb2RlLgorCiAyMDEzLTA1LTA2ICBHZW9mZnJleSBHYXJlbiAgPGdn YXJlbkBhcHBsZS5jb20+CiAKICAgICAgICAgQ2hlcnJ5LXBpY2sgbWVyZ2VkIHNvbWUgcGF0Y2hl cyB0byB0aGUgRlRMIGJyYW5jaC4KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZH U3BlY3VsYXRpdmVKSVQzMl82NC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRD b3JlL2RmZy9ERkdTcGVjdWxhdGl2ZUpJVDMyXzY0LmNwcAkocmV2aXNpb24gMTQ5NjQ5KQorKysg U291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdTcGVjdWxhdGl2ZUpJVDMyXzY0LmNwcAkod29y a2luZyBjb3B5KQpAQCAtNDIxNSw3ICs0MjE1LDcgQEAgdm9pZCBTcGVjdWxhdGl2ZUpJVDo6Y29t cGlsZShOb2RlKiBub2RlKQogICAgICAgICAKICAgICAgICAgV2F0Y2hwb2ludFNldCogd2F0Y2hw b2ludFNldCA9CiAgICAgICAgICAgICBtX2ppdC5nbG9iYWxPYmplY3RGb3Iobm9kZS0+Y29kZU9y aWdpbiktPnN5bWJvbFRhYmxlKCktPmdldCgKLSAgICAgICAgICAgICAgICBpZGVudGlmaWVyVUlE KG5vZGUtPmlkZW50aWZpZXJOdW1iZXJGb3JDaGVjaygpKS0+aW1wbCgpKS53YXRjaHBvaW50U2V0 KCk7CisgICAgICAgICAgICAgICAgaWRlbnRpZmllclVJRChub2RlLT5pZGVudGlmaWVyTnVtYmVy Rm9yQ2hlY2soKSkpLndhdGNocG9pbnRTZXQoKTsKICAgICAgICAgYWRkU2xvd1BhdGhHZW5lcmF0 b3IoCiAgICAgICAgICAgICBzbG93UGF0aENhbGwoCiAgICAgICAgICAgICAgICAgbV9qaXQuYnJh bmNoVGVzdDgoCkBAIC00MjQzLDcgKzQyNDMsNyBAQCB2b2lkIFNwZWN1bGF0aXZlSklUOjpjb21w aWxlKE5vZGUqIG5vZGUpCiAgICAgICAgIAogICAgIGNhc2UgR2xvYmFsVmFyV2F0Y2hwb2ludDog ewogICAgICAgICBXYXRjaHBvaW50U2V0KiBzZXQgPSBtX2ppdC5nbG9iYWxPYmplY3RGb3Iobm9k ZS0+Y29kZU9yaWdpbiktPnN5bWJvbFRhYmxlKCktPmdldCgKLSAgICAgICAgICAgIGlkZW50aWZp ZXJVSUQobm9kZS0+aWRlbnRpZmllck51bWJlckZvckNoZWNrKCkpLT5pbXBsKCkpLndhdGNocG9p bnRTZXQoKTsKKyAgICAgICAgICAgIGlkZW50aWZpZXJVSUQobm9kZS0+aWRlbnRpZmllck51bWJl ckZvckNoZWNrKCkpKS53YXRjaHBvaW50U2V0KCk7CiAgICAgICAgIG1faml0LmFkZExhemlseShz cGVjdWxhdGlvbldhdGNocG9pbnQoKSwgc2V0KTsKICAgICAgICAgCiAjaWYgREZHX0VOQUJMRShK SVRfQVNTRVJUKQpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9KSVRTdHVicy5jcHAK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9KSVRTdHVicy5jcHAJKHJl dmlzaW9uIDE0OTY0OSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9qaXQvSklUU3R1YnMuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC0xMzMsNjggKzEzMyw2IEBAIFNZTUJPTF9TVFJJTkcoY3RpT3BU aHJvd05vdENhdWdodCkgIjoiICIKICAgICAicmV0IiAiXG4iCiApOwogICAgIAotI2VsaWYgQ09N UElMRVIoR0NDKSAmJiBDUFUoWDg2XzY0KQotCi0vLyBUaGVzZSBBU1NFUlRzIHJlbWluZCB5b3Ug dGhhdCwgaWYgeW91IGNoYW5nZSB0aGUgbGF5b3V0IG9mIEpJVFN0YWNrRnJhbWUsIHlvdQotLy8g bmVlZCB0byBjaGFuZ2UgdGhlIGFzc2VtYmx5IHRyYW1wb2xpbmVzIGJlbG93IHRvIG1hdGNoLgot Q09NUElMRV9BU1NFUlQob2Zmc2V0b2Yoc3RydWN0IEpJVFN0YWNrRnJhbWUsIGNvZGUpICUgMzIg PT0gMHgwLCBKSVRTdGFja0ZyYW1lX21haW50YWluc18zMmJ5dGVfc3RhY2tfYWxpZ25tZW50KTsK LUNPTVBJTEVfQVNTRVJUKG9mZnNldG9mKHN0cnVjdCBKSVRTdGFja0ZyYW1lLCBzYXZlZFJCWCkg PT0gMHg0OCwgSklUU3RhY2tGcmFtZV9zdHViX2FyZ3VtZW50X3NwYWNlX21hdGNoZXNfY3RpVHJh bXBvbGluZSk7Ci1DT01QSUxFX0FTU0VSVChvZmZzZXRvZihzdHJ1Y3QgSklUU3RhY2tGcmFtZSwg Y2FsbEZyYW1lKSA9PSAweDkwLCBKSVRTdGFja0ZyYW1lX2NhbGxGcmFtZV9vZmZzZXRfbWF0Y2hl c19jdGlUcmFtcG9saW5lKTsKLUNPTVBJTEVfQVNTRVJUKG9mZnNldG9mKHN0cnVjdCBKSVRTdGFj a0ZyYW1lLCBjb2RlKSA9PSAweDgwLCBKSVRTdGFja0ZyYW1lX2NvZGVfb2Zmc2V0X21hdGNoZXNf Y3RpVHJhbXBvbGluZSk7Ci0KLWFzbSAoCi0iLmdsb2JsICIgU1lNQk9MX1NUUklORyhjdGlUcmFt cG9saW5lKSAiXG4iCi1ISURFX1NZTUJPTChjdGlUcmFtcG9saW5lKSAiXG4iCi1TWU1CT0xfU1RS SU5HKGN0aVRyYW1wb2xpbmUpICI6IiAiXG4iCi0gICAgInB1c2hxICVyYnAiICJcbiIKLSAgICAi bW92cSAlcnNwLCAlcmJwIiAiXG4iCi0gICAgInB1c2hxICVyMTIiICJcbiIKLSAgICAicHVzaHEg JXIxMyIgIlxuIgotICAgICJwdXNocSAlcjE0IiAiXG4iCi0gICAgInB1c2hxICVyMTUiICJcbiIK LSAgICAicHVzaHEgJXJieCIgIlxuIgotICAgICJzdWJxICQweDQ4LCAlcnNwIiAiXG4iCi0gICAg Im1vdnEgJDUxMiwgJXIxMiIgIlxuIgotICAgICJtb3ZxICQweEZGRkYwMDAwMDAwMDAwMDAsICVy MTQiICJcbiIKLSAgICAibW92cSAkMHhGRkZGMDAwMDAwMDAwMDAyLCAlcjE1IiAiXG4iCi0gICAg Im1vdnEgMHg5MCglcnNwKSwgJXIxMyIgIlxuIgotICAgICJjYWxsICoweDgwKCVyc3ApIiAiXG4i Ci0gICAgImFkZHEgJDB4NDgsICVyc3AiICJcbiIKLSAgICAicG9wcSAlcmJ4IiAiXG4iCi0gICAg InBvcHEgJXIxNSIgIlxuIgotICAgICJwb3BxICVyMTQiICJcbiIKLSAgICAicG9wcSAlcjEzIiAi XG4iCi0gICAgInBvcHEgJXIxMiIgIlxuIgotICAgICJwb3BxICVyYnAiICJcbiIKLSAgICAicmV0 IiAiXG4iCi0iLmdsb2JsICIgU1lNQk9MX1NUUklORyhjdGlUcmFtcG9saW5lRW5kKSAiXG4iCi1I SURFX1NZTUJPTChjdGlUcmFtcG9saW5lRW5kKSAiXG4iCi1TWU1CT0xfU1RSSU5HKGN0aVRyYW1w b2xpbmVFbmQpICI6IiAiXG4iCi0pOwotCi1hc20gKAotIi5nbG9ibCAiIFNZTUJPTF9TVFJJTkco Y3RpVk1UaHJvd1RyYW1wb2xpbmUpICJcbiIKLUhJREVfU1lNQk9MKGN0aVZNVGhyb3dUcmFtcG9s aW5lKSAiXG4iCi1TWU1CT0xfU1RSSU5HKGN0aVZNVGhyb3dUcmFtcG9saW5lKSAiOiIgIlxuIgot ICAgICJtb3ZxICVyc3AsICVyZGkiICJcbiIKLSAgICAiY2FsbCAiIExPQ0FMX1JFRkVSRU5DRShj dGlfdm1fdGhyb3cpICJcbiIKLSAgICAiaW50MyIgIlxuIgotKTsKLQotYXNtICgKLSIuZ2xvYmwg IiBTWU1CT0xfU1RSSU5HKGN0aU9wVGhyb3dOb3RDYXVnaHQpICJcbiIKLUhJREVfU1lNQk9MKGN0 aU9wVGhyb3dOb3RDYXVnaHQpICJcbiIKLVNZTUJPTF9TVFJJTkcoY3RpT3BUaHJvd05vdENhdWdo dCkgIjoiICJcbiIKLSAgICAiYWRkcSAkMHg0OCwgJXJzcCIgIlxuIgotICAgICJwb3BxICVyYngi ICJcbiIKLSAgICAicG9wcSAlcjE1IiAiXG4iCi0gICAgInBvcHEgJXIxNCIgIlxuIgotICAgICJw b3BxICVyMTMiICJcbiIKLSAgICAicG9wcSAlcjEyIiAiXG4iCi0gICAgInBvcHEgJXJicCIgIlxu IgotICAgICJyZXQiICJcbiIKLSk7Ci0KICNlbGlmIChDT01QSUxFUihHQ0MpIHx8IENPTVBJTEVS KFJWQ1QpKSAmJiBDUFUoQVJNX1RIVU1CMikKIAogI2RlZmluZSBUSFVOS19SRVRVUk5fQUREUkVT U19PRkZTRVQgICAgICAweDM4CkBAIC0yODksNyArMjI3LDkgQEAgZXh0ZXJuICJDIiB7CiAjZGVm aW5lIFJFR0lTVEVSX0ZJTEVfT0ZGU0VUICAgICAgICA5MgogI2RlZmluZSBHTE9CQUxfREFUQV9P RkZTRVQgICAgICAgICAxMDgKICNkZWZpbmUgU1RBQ0tfTEVOR1RIICAgICAgICAgICAgICAgMTEy CisKICNlbGlmIENQVShTSDQpCisKICNkZWZpbmUgU1lNQk9MX1NUUklORyhuYW1lKSAjbmFtZQog LyogY29kZSAocjQpLCBKU1N0YWNrKiAocjUpLCBDYWxsRnJhbWUqIChyNiksIHZvaWQqIHVudXNl ZDEgKHI3KSwgdm9pZCogdW51c2VkMihzcCksIFZNIChzcCkqLwogCkBAIC0zNjYsNyArMzA2LDcg QEAgU1lNQk9MX1NUUklORyhjdGlPcFRocm93Tm90Q2F1Z2h0KSAiOiIgIgogICAgICNlcnJvciAi SklUIG5vdCBzdXBwb3J0ZWQgb24gdGhpcyBwbGF0Zm9ybS4iCiAjZW5kaWYKIAotI2Vsc2UgLy8g VVNFKEpTVkFMVUUzMl82NCkKKyNlbHNlIC8vICFVU0UoSlNWQUxVRTMyXzY0KQogCiAjaWYgQ09N UElMRVIoR0NDKSAmJiBDUFUoWDg2XzY0KQogCkBAIC00NDgsNyArMzg4LDcgQEAgQ09NUElMRV9B U1NFUlQob2Zmc2V0b2Yoc3RydWN0IEpJVFN0YWNrRgogICAgICNlcnJvciAiSklUIG5vdCBzdXBw b3J0ZWQgb24gdGhpcyBwbGF0Zm9ybS4iCiAjZW5kaWYKIAotI2VuZGlmIC8vIFVTRShKU1ZBTFVF MzJfNjQpCisjZW5kaWYgLy8gIVVTRShKU1ZBTFVFMzJfNjQpCiAKICNpZiBDUFUoTUlQUykKIGFz bSAoCg==