[llvm] r185094 - Improve the compression of the tablegen DiffLists by introducing a new sort

Bill Schmidt wschmidt at linux.vnet.ibm.com
Fri Jun 28 08:49:35 PDT 2013


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
> 





More information about the llvm-commits mailing list