198014 2019-05-18 08:36:03 -0700 aarch64: ‘JSC::ARM64Assembler::LinkRecord::<unnamed union>::RealTypes::m_compareRegister’ is too small to hold all values of ‘JSC::ARM64Assembler::RegisterID’ {aka ‘enum JSC::ARM64Registers::RegisterID’} 2019-06-06 06:42:03 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build PC Linux RESOLVED FIXED P2 Normal --- 1 mcatanzaro mcatanzaro bugs-noreply commit-queue ews-watchlist fujii keith_miller mark.lam mcatanzaro msaboff saam webkit-bug-importer ysuzuki oldest_to_newest 1537412 0 mcatanzaro 2019-05-18 08:36:03 -0700 Building 2.24.1 (or maybe 2.24.2, not exactly sure tbh) on aarch64 we have a tremendous warning spam: [1452/2817] Building CXX object Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/__/__/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-02aa2997-2.cpp.o In file included from ../Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:30, from ../Source/JavaScriptCore/assembler/MacroAssembler.h:46, from ../Source/JavaScriptCore/runtime/BasicBlockLocation.h:30, from ../Source/JavaScriptCore/runtime/ControlFlowProfiler.h:29, from ../Source/JavaScriptCore/runtime/VM.h:35, from ../Source/JavaScriptCore/interpreter/CallFrame.h:30, from ../Source/JavaScriptCore/bytecode/VirtualRegister.h:29, from ../Source/JavaScriptCore/ftl/FTLExitArgumentForOperand.h:31, from ../Source/JavaScriptCore/ftl/FTLExitArgumentForOperand.cpp:27, from DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-02aa2997-2.cpp:1: ../Source/JavaScriptCore/assembler/ARM64Assembler.h:465:48: warning: ‘JSC::ARM64Assembler::LinkRecord::<unnamed union>::RealTypes::m_compareRegister’ is too small to hold all values of ‘JSC::ARM64Assembler::RegisterID’ {aka ‘enum JSC::ARM64Registers::RegisterID’} RegisterID m_compareRegister : 6; ^ 1541089 1 mcatanzaro 2019-06-02 07:15:40 -0700 This is making it extremely difficult to find build errors. 1542135 2 mcatanzaro 2019-06-05 13:24:03 -0700 CCing people who have touched this code before. Looks like this is a sequel to bug #135906. The enum is: typedef enum : int8_t { // Parameter/result registers. x0, x1, x2, x3, x4, x5, x6, x7, // Indirect result location register. x8, // Temporary registers. x9, x10, x11, x12, x13, x14, x15, // Intra-procedure-call scratch registers (temporary). x16, x17, // Platform Register (temporary). x18, // Callee-saved. x19, x20, x21, x22, x23, x24, x25, x26, x27, x28, // Special. fp, lr, sp, ip0 = x16, ip1 = x17, x29 = fp, x30 = lr, zr = 0x3f, // <-- problem is here InvalidGPRReg = -1, } RegisterID; The bitfield is six bits large, so only 32 values are allowed. But 0x3f is 63 decimal. It just doesn't fit. I think actually only 16 nonnegative values are allowed, because it's signed. So yeah, I don't understand how this could work properly without clobbering. Expanding the bitfield to seven bits is not enough. 8 bits makes the warning go away. We might not want it to be a bitfield at all at 8 bits, since it's an int8_t enum anyway, but just changing it to eight bits suffices. 1542141 3 mcatanzaro 2019-06-05 13:28:07 -0700 Ah, I tried just changing : 6 to : 8, but style checker hates it: ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:465: Please declare enum bitfields as unsigned integral types. [runtime/enum_bitfields] [5 Better not use a bitfield, then. 1542142 4 371434 mcatanzaro 2019-06-05 13:29:24 -0700 Created attachment 371434 Patch 1542179 5 371434 ysuzuki 2019-06-05 14:54:20 -0700 Comment on attachment 371434 Patch r=me. If we can shrink LinkRecord, we should represent the register in a bit fields, but in this case, it does not matter much, sizeof(LinkedRecord) does not change. 1542381 6 371434 commit-queue 2019-06-06 06:41:27 -0700 Comment on attachment 371434 Patch Clearing flags on attachment: 371434 Committed r246151: <https://trac.webkit.org/changeset/246151> 1542382 7 commit-queue 2019-06-06 06:41:29 -0700 All reviewed patches have been landed. Closing bug. 371434 2019-06-05 13:29:24 -0700 2019-06-06 06:41:27 -0700 Patch bug-198014-20190605152923.patch text/plain 2123 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ2MTE3CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA5 MTU3YzU0YTJkN2IzYTI0YTI0ZDU4NWRkMTNlZmViZGM1NWUzYjE3Li45YjYxYzY3ZDc1MzFlN2Nk YjM5OGUxM2FlMjBhNjFmMGQ2MDJiMjEyIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMCBAQAorMjAxOS0wNi0wNSAgTWljaGFlbCBDYXRhbnphcm8gIDxtY2F0YW56YXJvQGln YWxpYS5jb20+CisKKyAgICAgICAgYWFyY2g2NDog4oCYSlNDOjpBUk02NEFzc2VtYmxlcjo6TGlu a1JlY29yZDo6PHVubmFtZWQgdW5pb24+OjpSZWFsVHlwZXM6Om1fY29tcGFyZVJlZ2lzdGVy4oCZ IGlzIHRvbyBzbWFsbCB0byBob2xkIGFsbCB2YWx1ZXMgb2Yg4oCYSlNDOjpBUk02NEFzc2VtYmxl cjo6UmVnaXN0ZXJJROKAmSB7YWthIOKAmGVudW0gSlNDOjpBUk02NFJlZ2lzdGVyczo6UmVnaXN0 ZXJJROKAmX0KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTE5ODAxNAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFdoZW4gYnVpbGRpbmcgZm9yIGFhcmNoNjQsIHRoZXJlIGlzIGEgaHVnZSB3YXJuaW5nIHNwYW0g aGVyZS4gSXQncyBpbXBvc3NpYmxlIHRvIHNlZSBhbnkKKyAgICAgICAgb3RoZXIgd2FybmluZ3Mu IFRoaXMgaGFzIGJlZW4gb25nb2luZyBmb3Igc28gbG9uZyBJJ3ZlIGJlZ3VuIHRvIHN1c3BlY3Qg dGhhdCBub2JvZHkgd29ya3MKKyAgICAgICAgb24gdGhpcyBhcmNoaXRlY3R1cmUuCisKKyAgICAg ICAgQW55d2F5LCB0aGUgcHJvYmxlbSBpcyBiZWNhdXNlIHdlIG5lZWQgZWlnaHQgYml0cyB0byBz dG9yZSBhbGwgcG9zc2libGUgUmVnaXN0ZXJJRCB2YWx1ZXMsCisgICAgICAgIGJ1dCB0aGUgYml0 ZmllbGQgaXMgb25seSBzaXggYml0cyB3aWRlLiBGaXggaXQuIFRoZSBDT01QSUxFX0FTU0VSVCBj aGVja2luZyB0aGUgc2l6ZSBvZiB0aGlzCisgICAgICAgIHN0cnVjdCBpcyBzdGlsbCBoYXBweSwg c28gSSBwcmVzdW1lIHRoZSBjaGFuZ2UgaXMgT0suCisKKyAgICAgICAgKiBhc3NlbWJsZXIvQVJN NjRBc3NlbWJsZXIuaDoKKwogMjAxOS0wNi0wNCAgTWljaGFlbCBDYXRhbnphcm8gIDxtY2F0YW56 YXJvQGlnYWxpYS5jb20+CiAKICAgICAgICAgRml4IG1pc2NlbGxhbmVvdXMgYnVpbGQgd2Fybmlu Z3MKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvQVJNNjRBc3Nl bWJsZXIuaCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvQVJNNjRBc3NlbWJsZXIu aAppbmRleCA2ZmQ2MjhhMjU4NjZmYjRmYjk2ZmQ1MGExNGQwNzRiNDk1ZmQ0ODdlLi41NDRiNjEy YzM0NWJlOTU5NjY5YjFhMjc2ZGU1NWExZTJkNDg4MjUwIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvYXNzZW1ibGVyL0FSTTY0QXNzZW1ibGVyLmgKKysrIGIvU291cmNlL0phdmFT Y3JpcHRDb3JlL2Fzc2VtYmxlci9BUk02NEFzc2VtYmxlci5oCkBAIC00NTgsMTEgKzQ1OCwxMSBA QCBwdWJsaWM6CiAgICAgICAgICAgICBzdHJ1Y3QgUmVhbFR5cGVzIHsKICAgICAgICAgICAgICAg ICBpbnQ2NF90IG1fZnJvbTsKICAgICAgICAgICAgICAgICBpbnQ2NF90IG1fdG87CisgICAgICAg ICAgICAgICAgUmVnaXN0ZXJJRCBtX2NvbXBhcmVSZWdpc3RlcjsKICAgICAgICAgICAgICAgICBK dW1wVHlwZSBtX3R5cGUgOiA4OwogICAgICAgICAgICAgICAgIEp1bXBMaW5rVHlwZSBtX2xpbmtU eXBlIDogODsKICAgICAgICAgICAgICAgICBDb25kaXRpb24gbV9jb25kaXRpb24gOiA0OwogICAg ICAgICAgICAgICAgIHVuc2lnbmVkIG1fYml0TnVtYmVyIDogNjsKLSAgICAgICAgICAgICAgICBS ZWdpc3RlcklEIG1fY29tcGFyZVJlZ2lzdGVyIDogNjsKICAgICAgICAgICAgICAgICBib29sIG1f aXM2NEJpdCA6IDE7CiAgICAgICAgICAgICB9IHJlYWxUeXBlczsKICAgICAgICAgICAgIHN0cnVj dCBDb3B5VHlwZXMgewo=