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

Wed Nov 3 21:53:30 PDT 2021

dblaikie added a comment.

In D112105#3107923 <https://reviews.llvm.org/D112105#3107923>, @stellaraccident wrote:

> 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.

Ah, thanks for the context!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112105