[llvm] r185094 - Improve the compression of the tablegen DiffLists by introducing a new sort
Chad Rosier
mcrosier at apple.com
Fri Jun 28 11:14:25 PDT 2013
Thanks, Ben! I'm able to reproduce the issue and am investigating (3d)now.
Chad
On Jun 28, 2013, at 10:27 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>
> On 28.06.2013, at 18:54, Chad Rosier <mcrosier at apple.com> wrote:
>
>> Hi Bill,
>> I'm not seeing the failure on my local machine, but I do see some suspect code.
>
> It reproduces on intel if you add "-mcpu=generic" to the run line. PPC and Mips use that as a fallback, x86 always uses the native CPU.
>
> - Ben
>
>>
>> Just below the assert in X86FloatingPoint.cpp::setupBlockStack() I see the following code:
>> ----------
>> // Push the fixed live-in registers.
>> for (unsigned i = Bundle.FixCount; i > 0; --i) {
>> MBB->addLiveIn(X86::ST0+i-1);
>> DEBUG(dbgs() << "Live-in st(" << (i-1) << "): %FP"
>> << unsigned(Bundle.FixStack[i-1]) << '\n');
>> pushReg(Bundle.FixStack[i-1]);
>> }
>> ----------
>>
>> The X86::ST0+i-1 is doing ugly enumeration math, which makes assumptions about how the registers are enumerated.
>>
>> Sadly, I see a few other places in the code doing similar tricks.
>>
>> I'll see if I can't come up with a solution.
>>
>> Chad
>>
>>
>> On Jun 28, 2013, at 8:49 AM, Bill Schmidt <wschmidt at linux.vnet.ibm.com> wrote:
>>
>>> Hi Chad,
>>>
>>> This commit appears to have introduced a regression on PPC64 and Mips:
>>>
>>> FAIL: LLVM :: CodeGen/X86/3dnow-intrinsics.ll (9708 of 14686)
>>> ******************** TEST 'LLVM :: CodeGen/X86/3dnow-intrinsics.ll'
>>> FAILED ********************
>>> Script:
>>> --
>>> /home/wschmidt/llvm/build/llvm-base/Release+Asserts/bin/llc
>>> < /home/wschmidt/llvm/llvm-base/test/CodeGen/X86/3dnow-intrinsics.ll
>>> -march=x86 -mattr=+3dnow | /home/wschmidt/llvm/build/llvm-base/Release
>>> +Asserts/bin/FileCheck /home/wschmidt/llvm/llvm-base/test/CodeGen/X86/3dnow-intrinsics.ll
>>> --
>>> Exit Code: 2
>>> Command Output (stderr):
>>> --
>>> llc: /home/wschmidt/llvm/llvm-base/lib/Target/X86/X86FloatingPoint.cpp:510: void {anonymous}::FPS::setupBlockStack(): Assertion `Bundle.isFixed() && "Reached block before any predecessors"' failed.
>>> 0 llc 0x0000000011010ae0 llvm::sys::PrintStackTrace(_IO_FILE*) +
>>> 4286609640
>>> 1 llc 0x0000000011010d3c
>>> 2 llc 0x0000000011010680
>>> 3 0x00000fffb0a60418 __kernel_sigtramp_rt64 + 0
>>> 4 libc.so.6 0x00000fffb0539f38 abort + 4293567192
>>> 5 libc.so.6 0x00000fffb052e9b8 __assert_fail + 4293526536
>>> 6 llc 0x00000000106a9698
>>> 7 llc 0x00000000106aad24
>>> 8 llc 0x0000000010a23168
>>> llvm::MachineFunctionPass::runOnFunction(llvm::Function&) + 4280722504
>>> 9 llc 0x0000000010f8d9b8
>>> llvm::FPPassManager::runOnFunction(llvm::Function&) + 4286105216
>>> 10 llc 0x0000000010f8da2c
>>> llvm::FPPassManager::runOnModule(llvm::Module&) + 4286105308
>>> 11 llc 0x0000000010f8d2e8
>>> llvm::MPPassManager::runOnModule(llvm::Module&) + 4286103544
>>> 12 llc 0x0000000010f8d584
>>> llvm::PassManagerImpl::run(llvm::Module&) + 4286104188
>>> 13 llc 0x0000000010f8d684 llvm::PassManager::run(llvm::Module&) +
>>> 4286104420
>>> 14 llc 0x00000000101c8a04
>>> 15 llc 0x00000000101b9640 main + 4272228752
>>> 16 libc.so.6 0x00000fffb051f07c
>>> 17 libc.so.6 0x00000fffb051f29c __libc_start_main + 4293465508
>>> Stack dump:
>>> 0. Program arguments: /home/wschmidt/llvm/build/llvm-base/Release
>>> +Asserts/bin/llc -march=x86 -mattr=+3dnow
>>> 1. Running pass 'Function Pass Manager' on module '<stdin>'.
>>> 2. Running pass 'X86 FP Stackifier' on function '@test_pi2fd'
>>> FileCheck error: '-' is empty.
>>> --
>>>
>>> Thanks,
>>> Bill
>>>
>>>
>>> On Thu, 2013-06-27 at 19:38 +0000, Chad Rosier wrote:
>>>> Author: mcrosier
>>>> Date: Thu Jun 27 14:38:13 2013
>>>> New Revision: 185094
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=185094&view=rev
>>>> Log:
>>>> Improve the compression of the tablegen DiffLists by introducing a new sort
>>>> algorithm when assigning EnumValues to the synthesized registers.
>>>>
>>>> The current algorithm, LessRecord, uses the StringRef compare_numeric
>>>> function. This function compares strings, while handling embedded numbers.
>>>> For example, the R600 backend registers are sorted as follows:
>>>>
>>>> T1
>>>> T1_W
>>>> T1_X
>>>> T1_XYZW
>>>> T1_Y
>>>> T1_Z
>>>> T2
>>>> T2_W
>>>> T2_X
>>>> T2_XYZW
>>>> T2_Y
>>>> T2_Z
>>>>
>>>> In this example, the 'scaling factor' is dEnum/dN = 6 because T0, T1, T2
>>>> have an EnumValue offset of 6 from one another. However, in other parts
>>>> of the register bank, the scaling factors are different:
>>>>
>>>> dEnum/dN = 5:
>>>> KC0_128_W
>>>> KC0_128_X
>>>> KC0_128_XYZW
>>>> KC0_128_Y
>>>> KC0_128_Z
>>>> KC0_129_W
>>>> KC0_129_X
>>>> KC0_129_XYZW
>>>> KC0_129_Y
>>>> KC0_129_Z
>>>>
>>>> The diff lists do not work correctly because different kinds of registers have
>>>> different 'scaling factors'. This new algorithm, LessRecordRegister, tries to
>>>> enforce a scaling factor of 1. For example, the registers are now sorted as
>>>> follows:
>>>>
>>>> T1
>>>> T2
>>>> T3
>>>> ...
>>>> T0_W
>>>> T1_W
>>>> T2_W
>>>> ...
>>>> T0_X
>>>> T1_X
>>>> T2_X
>>>> ...
>>>> KC0_128_W
>>>> KC0_129_W
>>>> KC0_130_W
>>>> ...
>>>>
>>>> For the Mips and R600 I see a 19% and 6% reduction in size, respectively. I
>>>> did see a few small regressions, but the differences were on the order of a
>>>> few bytes (e.g., AArch64 was 16 bytes). I suspect there will be even
>>>> greater wins for targets with larger register files.
>>>>
>>>> Patch reviewed by Jakob.
>>>> rdar://14006013
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/TableGen/Record.h
>>>> llvm/trunk/test/MC/ARM/basic-arm-instructions.s
>>>> llvm/trunk/utils/TableGen/CodeGenRegisters.cpp
>>>> llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp
>>>>
>>>> Modified: llvm/trunk/include/llvm/TableGen/Record.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/TableGen/Record.h?rev=185094&r1=185093&r2=185094&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/TableGen/Record.h (original)
>>>> +++ llvm/trunk/include/llvm/TableGen/Record.h Thu Jun 27 14:38:13 2013
>>>> @@ -1731,6 +1731,86 @@ struct LessRecordFieldName {
>>>> }
>>>> };
>>>>
>>>> +struct LessRecordRegister {
>>>> + static size_t min(size_t a, size_t b) { return a < b ? a : b; }
>>>> + static bool ascii_isdigit(char x) { return x >= '0' && x <= '9'; }
>>>> +
>>>> + struct RecordParts {
>>>> + SmallVector<std::pair< bool, StringRef>, 4> Parts;
>>>> +
>>>> + RecordParts(StringRef Rec) {
>>>> + if (Rec.empty())
>>>> + return;
>>>> +
>>>> + size_t Len = 0;
>>>> + const char *Start = Rec.data();
>>>> + const char *Curr = Start;
>>>> + bool isDigitPart = ascii_isdigit(Curr[0]);
>>>> + for (size_t I = 0, E = Rec.size(); I != E; ++I, ++Len) {
>>>> + bool isDigit = ascii_isdigit(Curr[I]);
>>>> + if (isDigit != isDigitPart) {
>>>> + Parts.push_back(std::make_pair(isDigitPart, StringRef(Start, Len)));
>>>> + Len = 0;
>>>> + Start = &Curr[I];
>>>> + isDigitPart = ascii_isdigit(Curr[I]);
>>>> + }
>>>> + }
>>>> + // Push the last part.
>>>> + Parts.push_back(std::make_pair(isDigitPart, StringRef(Start, Len)));
>>>> + }
>>>> +
>>>> + size_t size() { return Parts.size(); }
>>>> +
>>>> + std::pair<bool, StringRef> getPart(size_t i) {
>>>> + assert (i < Parts.size() && "Invalid idx!");
>>>> + return Parts[i];
>>>> + }
>>>> + };
>>>> +
>>>> + bool operator()(const Record *Rec1, const Record *Rec2) const {
>>>> + RecordParts LHSParts(StringRef(Rec1->getName()));
>>>> + RecordParts RHSParts(StringRef(Rec2->getName()));
>>>> +
>>>> + size_t LHSNumParts = LHSParts.size();
>>>> + size_t RHSNumParts = RHSParts.size();
>>>> + assert (LHSNumParts && RHSNumParts && "Expected at least one part!");
>>>> +
>>>> + if (LHSNumParts != RHSNumParts)
>>>> + return LHSNumParts < RHSNumParts;
>>>> +
>>>> + // We expect the registers to be of the form [_a-zA-z]+([0-9]*[_a-zA-Z]*)*.
>>>> + for (size_t I = 0, E = LHSNumParts; I < E; I+=2) {
>>>> + std::pair<bool, StringRef> LHSPart = LHSParts.getPart(I);
>>>> + std::pair<bool, StringRef> RHSPart = RHSParts.getPart(I);
>>>> + if ((I & 1) == 0) { // Expect even part to always be alpha.
>>>> + assert (LHSPart.first == false && RHSPart.first == false &&
>>>> + "Expected both parts to be alpha.");
>>>> + if (int Res = LHSPart.second.compare(RHSPart.second))
>>>> + return Res < 0;
>>>> + }
>>>> + }
>>>> + for (size_t I = 1, E = LHSNumParts; I < E; I+=2) {
>>>> + std::pair<bool, StringRef> LHSPart = LHSParts.getPart(I);
>>>> + std::pair<bool, StringRef> RHSPart = RHSParts.getPart(I);
>>>> + if (I & 1) { // Expect odd part to always be numeric.
>>>> + assert (LHSPart.first == true && RHSPart.first == true &&
>>>> + "Expected both parts to be numeric.");
>>>> + if (LHSPart.second.size() != RHSPart.second.size())
>>>> + return LHSPart.second.size() < RHSPart.second.size();
>>>> +
>>>> + unsigned LHSVal, RHSVal;
>>>> + if (LHSPart.second.getAsInteger(10, LHSVal))
>>>> + assert("Unable to convert LHS to integer.");
>>>> + if (RHSPart.second.getAsInteger(10, RHSVal))
>>>> + assert("Unable to convert RHS to integer.");
>>>> + if (LHSVal != RHSVal)
>>>> + return LHSVal < RHSVal;
>>>> + }
>>>> + }
>>>> + return LHSNumParts < RHSNumParts;
>>>> + }
>>>> +};
>>>> +
>>>> raw_ostream &operator<<(raw_ostream &OS, const RecordKeeper &RK);
>>>>
>>>> /// QualifyName - Return an Init with a qualifier prefix referring
>>>>
>>>> Modified: llvm/trunk/test/MC/ARM/basic-arm-instructions.s
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/basic-arm-instructions.s?rev=185094&r1=185093&r2=185094&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/test/MC/ARM/basic-arm-instructions.s (original)
>>>> +++ llvm/trunk/test/MC/ARM/basic-arm-instructions.s Thu Jun 27 14:38:13 2013
>>>> @@ -897,17 +897,17 @@ Lforward:
>>>> ldm r0, {r0, r2, lr}^
>>>> ldm sp!, {r0-r3, pc}^
>>>>
>>>> -@ CHECK: ldm r2, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x92,0xe8]
>>>> -@ CHECK: ldm r2, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x92,0xe8]
>>>> -@ CHECK: ldmib r2, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x92,0xe9]
>>>> -@ CHECK: ldmda r2, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x12,0xe8]
>>>> -@ CHECK: ldmdb r2, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x12,0xe9]
>>>> -@ CHECK: ldm r2, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x92,0xe8]
>>>> -
>>>> -@ CHECK: ldm r2!, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0xb2,0xe8]
>>>> -@ CHECK: ldmib r2!, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0xb2,0xe9]
>>>> -@ CHECK: ldmda r2!, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x32,0xe8]
>>>> -@ CHECK: ldmdb r2!, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x32,0xe9]
>>>> +@ CHECK: ldm r2, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x92,0xe8]
>>>> +@ CHECK: ldm r2, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x92,0xe8]
>>>> +@ CHECK: ldmib r2, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x92,0xe9]
>>>> +@ CHECK: ldmda r2, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x12,0xe8]
>>>> +@ CHECK: ldmdb r2, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x12,0xe9]
>>>> +@ CHECK: ldm r2, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x92,0xe8]
>>>> +
>>>> +@ CHECK: ldm r2!, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0xb2,0xe8]
>>>> +@ CHECK: ldmib r2!, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0xb2,0xe9]
>>>> +@ CHECK: ldmda r2!, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x32,0xe8]
>>>> +@ CHECK: ldmdb r2!, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x32,0xe9]
>>>> @ CHECK: ldm r0, {lr, r0, r2} ^ @ encoding: [0x05,0x40,0xd0,0xe8]
>>>> @ CHECK: ldm sp!, {pc, r0, r1, r2, r3} ^ @ encoding: [0x0f,0x80,0xfd,0xe8]
>>>>
>>>> @@ -2332,17 +2332,17 @@ Lforward:
>>>> stmda sp!, {r1,r3-r6}
>>>> stmdb r0!, {r1,r5,r7,sp}
>>>>
>>>> -@ CHECK: stm r2, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x82,0xe8]
>>>> +@ CHECK: stm r2, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x82,0xe8]
>>>> @ CHECK: stm r3, {lr, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x40,0x83,0xe8]
>>>> -@ CHECK: stmib r4, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x84,0xe9]
>>>> -@ CHECK: stmda r5, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x05,0xe8]
>>>> +@ CHECK: stmib r4, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x84,0xe9]
>>>> +@ CHECK: stmda r5, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x05,0xe8]
>>>> @ CHECK: stmdb r6, {r1, r3, r4, r5, r6, r8} @ encoding: [0x7a,0x01,0x06,0xe9]
>>>> -@ CHECK: stmdb sp, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0x0d,0xe9]
>>>> +@ CHECK: stmdb sp, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0x0d,0xe9]
>>>>
>>>> -@ CHECK: stm r8!, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0xa8,0xe8]
>>>> -@ CHECK: stmib r9!, {r1, r3, r4, r5, r6, sp} @ encoding: [0x7a,0x20,0xa9,0xe9]
>>>> +@ CHECK: stm r8!, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0xa8,0xe8]
>>>> +@ CHECK: stmib r9!, {sp, r1, r3, r4, r5, r6} @ encoding: [0x7a,0x20,0xa9,0xe9]
>>>> @ CHECK: stmda sp!, {r1, r3, r4, r5, r6} @ encoding: [0x7a,0x00,0x2d,0xe8]
>>>> -@ CHECK: stmdb r0!, {r1, r5, r7, sp} @ encoding: [0xa2,0x20,0x20,0xe9]
>>>> +@ CHECK: stmdb r0!, {sp, r1, r5, r7} @ encoding: [0xa2,0x20,0x20,0xe9]
>>>>
>>>>
>>>> @------------------------------------------------------------------------------
>>>>
>>>> Modified: llvm/trunk/utils/TableGen/CodeGenRegisters.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenRegisters.cpp?rev=185094&r1=185093&r2=185094&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/utils/TableGen/CodeGenRegisters.cpp (original)
>>>> +++ llvm/trunk/utils/TableGen/CodeGenRegisters.cpp Thu Jun 27 14:38:13 2013
>>>> @@ -938,7 +938,7 @@ CodeGenRegBank::CodeGenRegBank(RecordKee
>>>>
>>>> // Read in the register definitions.
>>>> std::vector<Record*> Regs = Records.getAllDerivedDefinitions("Register");
>>>> - std::sort(Regs.begin(), Regs.end(), LessRecord());
>>>> + std::sort(Regs.begin(), Regs.end(), LessRecordRegister());
>>>> Registers.reserve(Regs.size());
>>>> // Assign the enumeration values.
>>>> for (unsigned i = 0, e = Regs.size(); i != e; ++i)
>>>> @@ -947,10 +947,16 @@ CodeGenRegBank::CodeGenRegBank(RecordKee
>>>> // Expand tuples and number the new registers.
>>>> std::vector<Record*> Tups =
>>>> Records.getAllDerivedDefinitions("RegisterTuples");
>>>> +
>>>> + std::vector<Record*> TupRegsCopy;
>>>> for (unsigned i = 0, e = Tups.size(); i != e; ++i) {
>>>> const std::vector<Record*> *TupRegs = Sets.expand(Tups[i]);
>>>> - for (unsigned j = 0, je = TupRegs->size(); j != je; ++j)
>>>> - getReg((*TupRegs)[j]);
>>>> + TupRegsCopy.reserve(TupRegs->size());
>>>> + TupRegsCopy.assign(TupRegs->begin(), TupRegs->end());
>>>> + std::sort(TupRegsCopy.begin(), TupRegsCopy.end(), LessRecordRegister());
>>>> + for (unsigned j = 0, je = TupRegsCopy.size(); j != je; ++j)
>>>> + getReg((TupRegsCopy)[j]);
>>>> + TupRegsCopy.clear();
>>>> }
>>>>
>>>> // Now all the registers are known. Build the object graph of explicit
>>>>
>>>> Modified: llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp?rev=185094&r1=185093&r2=185094&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp (original)
>>>> +++ llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp Thu Jun 27 14:38:13 2013
>>>> @@ -309,7 +309,7 @@ RegisterInfoEmitter::EmitRegMappingTable
>>>> const std::vector<CodeGenRegister*> &Regs,
>>>> bool isCtor) {
>>>> // Collect all information about dwarf register numbers
>>>> - typedef std::map<Record*, std::vector<int64_t>, LessRecord> DwarfRegNumsMapTy;
>>>> + typedef std::map<Record*, std::vector<int64_t>, LessRecordRegister> DwarfRegNumsMapTy;
>>>> DwarfRegNumsMapTy DwarfRegNums;
>>>>
>>>> // First, just pull all provided information to the map
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list