[PATCH] D111688: [MachineSink] Compile time improvement for large testcases which has many kill flags

Bing Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 19 19:03:00 PDT 2021


yubing added a comment.

In D111688#3074036 <https://reviews.llvm.org/D111688#3074036>, @craig.topper wrote:

> In D111688#3062166 <https://reviews.llvm.org/D111688#3062166>, @MatzeB wrote:
>
>> huh... I guess the problem here was that we interpreted the register numbers as plain unsigned... And virtual registers always have bit 31 set, so I guess the bitset could indeed grow to unreasonable sizes.
>>
>> - Please try if `DenseSet<Register>` works too.
>> - If you can please find a shorter more succinct title.
>>
>> Then we should be good to land this.
>
> SparseBitVector shouldn't be effected by bit 31 being set. It stores 128 bit chunks of bits in a linked list. Insertion does a linear scan forward or backward from the most recently accessed chunk to trying to find the chunk to insert in. Were we accessing it in some pathologically bad way that caused long linear scans?
>
> If the issue is with the inserting function, then the title of this patch is misleading. The number of kill flags in is irrelevant. The place where the insert happens doesn't know how many kill flags exist. Only the later call to MRI->clearKillFlags(I) would know that.

Hi, Craig.
Yeah, title is misleading. There are many regs whose kill flags need to be cleared, and machinesink visit these regs such randomly that  SparseBitVector is not fit for storing info of these regs.
So I guess a better title is "[MachineSink] Compile time improvement for large testcases which has many regs whose kill flags need to be cleared"?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111688/new/

https://reviews.llvm.org/D111688



More information about the llvm-commits mailing list