[PATCH] D129396: [InstCombine] Add support for div, ldiv, lldiv, and imaxdiv folding

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D129396#3640028 <https://reviews.llvm.org/D129396#3640028>, @msebor wrote:

> Thanks for the pointer to D129396 <https://reviews.llvm.org/D129396>!
>
> I didn't expect calling convention differences to be visible this early in the middle end (they're not in GCC).  That seems unfortunate, but at least I learned something new.  That said, on some of the targets mentioned in D129396 <https://reviews.llvm.org/D129396> such as arm or i386, `LibCallSimplifier::optimizeDiv` isn't invoked for calls to `div` because they pass the function three arguments rather than the expected two (a pointer to `struct div_t` to fill with the result, followed by the two expected integers), and so those calls are not recognized (`TargetLibraryInfo::getLibFunc()` returns `false`).  So while those targets don't benefit from the change I wouldn't expect them to be adversely affected by it.  Unless this isn't the only other calling convention, presumably support for it could be added in a follow-up.

In my first comment above I shared two examples that use a different two argument ABI. In one case, `i64` is used instead of `{ i32, i32 }` and in the other `[2 x i64]` is used instead of `{ i64, i64 }`. I would expect your current code to crash for this due to a RAUW type mismatch. Now, you could add a return type check to getLibFunc() and ensure that the return type is indeed a struct, in which case I think your code would work for the platforms that do use the straightforward ABI.

Solving this on the Clang side sounds a good bit more robust, but I'm not familiar with how that would look like from a technical perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129396