[PATCH] D112105: [ADT] Simplifying hex string parsing so it runs faster in debug modes.

Stella Laurenzo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 3 20:27:37 PDT 2021


stellaraccident added a comment.

In D112105#3079880 <https://reviews.llvm.org/D112105#3079880>, @dblaikie wrote:

> In D112105#3079869 <https://reviews.llvm.org/D112105#3079869>, @stellaraccident wrote:
>
>> In D112105#3076917 <https://reviews.llvm.org/D112105#3076917>, @dblaikie wrote:
>>
>>> We don't usually optimize for debug performance - is there a particular case where this comes up that's worth prioritizing/changing code for (& is there a good heuristic we can use about which debug performance is going to matter and which isn't?)
>>
>> Interested who the "we" is. Debug builds should be generally usable and this case was extreme, pushing things into unusable for very common programs.
>
> My general understanding of the LLVM community - which is a fluid thing, to be sure. But insofar as any of us try to represent that amorphous set of values, that's what I'm trying to convey here.
>
> (generally if someone else has ongoing/outstanding concerns over code, we also don't usually approve a patch in spite of that - or at least leave a "OK so long as <other person's> concerns are addressed" (I usually don't use "approve" in that case, I leave it to the other person to provide the final approval so as not to confuse things))
>
> But I'll leave this to you folks - as much as a bunch of the substance and process here doesn't sit well with me.

I approved the patch because I believed it was worthy to approve. I feel like that is completely in bounds. Landing it without consensus would have been out of bounds - and the discussion was clearly still going on. I could have stated the intention more clearly but it was not meant to bypass.

In any case, thanks for the review and sorry for confusion. The author doesn't have commit access, so I'll land this.


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

https://reviews.llvm.org/D112105



More information about the llvm-commits mailing list