[PATCH] D106450: [InstCombine] Fold (gep (oneuse(gep Ptr, Idx0)), Idx1) -> (gep Ptr, (add Idx0, Idx1)) (PR51069)

Mon Aug 23 10:57:30 PDT 2021

echristo added a comment.

In D106450#2960467 <https://reviews.llvm.org/D106450#2960467>, @xbolva00 wrote:

> In D106450#2960400 <https://reviews.llvm.org/D106450#2960400>, @alexfh wrote:
>
>> In D106450#2956763 <https://reviews.llvm.org/D106450#2956763>, @RKSimon wrote:
>>
>>> In D106450#2956001 <https://reviews.llvm.org/D106450#2956001>, @alexfh wrote:
>>>
>>>> In D106450#2955083 <https://reviews.llvm.org/D106450#2955083>, @aeubanks wrote:
>>>>
>>>>> Given that we have a test case and multiple people have reported regressions, can we revert in the meantime?
>>>>
>>>> Especially if there's no obvious and clear forward fix, I'd appreciate if you could unblock us by reverting for now. Thanks!
>>>
>>> Except that regresses other benchmarks that I was addressing with this patch - bullet etc.
>>
>> Is the regression fixed by this patch more serious than the one introduced by it? Is it clear on how to fix the new regression?
>
> I believe Bullet > “two internal *micro*benchmarks”

FWIW it's not a micro benchmark per se. It's basically compression/decompression algorithms around this code base: https://github.com/google/snappy, the particular data corpus is internal, but it was significant enough and widespread across a number of other benchmarks as well. In addition it seemed to exist across multiple microarchitectures (and we've seen it on Power above and I'm going to hypothesize we'd see it on ARM). Simon's comments feel right around where the problem is likely coming from as well.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106450