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

Wed May 5 22:46:47 PDT 2021

dvyukov added a comment.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101536