[PATCH] D89955: [ValueTracking] Recognize more integer abs idioms

Tue Oct 27 03:48:51 PDT 2020

frasercrmck added a comment.

In D89955#2355653 <https://reviews.llvm.org/D89955#2355653>, @lebedev.ri wrote:

> Please can you explain the bigger problem here?
> I'm not sure why the solution isn't: "just run the middle-end optimization passes, or don't expect good back-end codegen".

The original motivation was that when we moved from LLVM7 to LLVM9 we observed a degradation in the recognition of abs idioms (using clang, so including InstCombine @shchenz). I tracked it to the time at which abs idioms were moved out of DAGCombine and into the initial lowering from IR, where I found a potential for more patterns.  It's possible something happened to our code in between InstCombine and ISel as our target had support for i8/i16 but only when extended/truncated to/from i32: exts & truncs were common and it was generally awkward in LLVM. Sadly this is all hypothetical since I've lost access to the code so I can't get at the full reproduction details.

I put this patch up anyway, since I thought that the ValueTracking pattern analysis had a function outside of the specific position(s) in the pipeline it's currently run. I reasoned that you might want to match this select pattern before InstCombine, for instance. If that's not ValueTracking's agreed-upon contract (which I'd understand since then its scope becomes much larger: it has to settle on some form of canonicalization) then your solution is the one we should take. It sounds to me like a question of determining which pass is responsible for what.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89955