[PATCH] D101536: [SimplifyCFG] Common code sinking: drop profitability check in presence of conditional predecessors (PR30244)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D101536#2741191 <https://reviews.llvm.org/D101536#2741191>, @dvyukov wrote:

> In D101536#2740351 <https://reviews.llvm.org/D101536#2740351>, @lebedev.ri wrote:
>
>> In D101536#2738069 <https://reviews.llvm.org/D101536#2738069>, @dvyukov wrote:
>>
>>> In D101536#2737140 <https://reviews.llvm.org/D101536#2737140>, @lebedev.ri wrote:
>>>
>>>> @dvyukov Hi! Would it please be possible for you to redo the two-stage reproduction process
>>>> you stated in https://bugs.llvm.org/show_bug.cgi?id=30244
>>>> to double-check that i'm not simply doing it wrong, and the issue is/isn't there still?
>>>
>>> I trust you to re-do the check. It's not that I can do it better or would trust myself 5 years later :)
>>> I see you added some unit tests, so that should be good enough.
>>
>> Oh well, i was hoping for a sanity check :S
>> I've already tried it twice locally, and as far as i can tell, this doesn't cause any similar regression.
>>
>> So how should we proceed here?
>> Speculatively land and see if some other perf regression is reported against this change?
>
> I tried to figure out if compiler-rt/lib/tsan/check_analyze.sh is still executed as part of testing, I remember some discussions on removing it.
> But I didn't where it's executed, nor where it was removed. But I see e.g. number of pop's check was removed over time. So I am not sure we can still rely on it.

I was mainly talking about https://bugs.llvm.org/show_bug.cgi?id=30244#c1, the reproducible run-time slowdown.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101536