[PATCH] D143631: [LTO] Don't let InstCombine re-sink the vastly more expensive fdiv

Mon Feb 13 07:57:58 PST 2023

craig.topper added a comment.

In D143631#4123030 <https://reviews.llvm.org/D143631#4123030>, @spatel wrote:

> In D143631#4119689 <https://reviews.llvm.org/D143631#4119689>, @craig.topper wrote:
>
>> I've added another variation I encountered to D87479 <https://reviews.llvm.org/D87479>  which another run of LICM will not fix.
>
> Would that case be fixed by disabling sinking in InstCombine? Maybe it's finally time to just drop sinking in InstCombine and deal with any fallout.
>
> At least as a first check, compile-time sees a net improvement:
> https://llvm-compile-time-tracker.com/compare.php?from=d87468e56c221555be7a1c1550036af5b7034896&to=a5691dd50d4098dec45571fcdfdc5403826a07de&stat=instructions:u
>
> That change should also allow us to close a pile of longstanding InstCombine bugs caused by a conflict between iterative sinking and InstCombine's infinite-loop-threshold.
>
> I get ~60 regression test failures with that patch, but regenerating the first 5 or so tests that I tried to update does not show any real regressions.

I don't think my new variation as anything to do with the sinking code in InstCombine. It's being caused by the same transform D87479 <https://reviews.llvm.org/D87479> touched. That code creates an fmul followed by fdiv at the location of the original fmul. Even if the original fdiv was not in the same basic block or loop as the fmul. LICM will hoist a reciprocal out later, but that leaves an extra fmul.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143631