[PATCH] D114174: [ARM][CodeGen] Add support for complex addition and multiplication

[Joshua Cranmer via Phabricator via llvm-commits]

jcranmer-intel added a comment.

I haven't delved into the ARM-specific code in detail, but the `ComplexArithmeticGraph` feels like it's reinventing a lot of Instruction-like infrastructure just to avoid having to do anything with complex intrinsics. I //may// be biased here in thinking that it would be be better to move to standardized complex intrinsics, and I understand why you don't want to go there, but there are two questions I have:

- What operations would you need from standardized complex intrinsics to completely eliminate all of logic in `ComplexArithmeticGraph`?
- Have you tried an interface that lets targets nominate target-specific intrinsics for complex operations in lieu of creating a new graph ISA?



================
Comment at: llvm/include/llvm/Transforms/Scalar/ComplexArithmetic.h:34
+ */
+class ComplexArithmeticGraph {
+public:
----------------
This says `ComplexArithmetic`, but it's mostly just limited to complex multiplication, right? There's no support for complex division or absolute value that I see (not that complex division is implemented in any hardware I'm aware of).


================
Comment at: llvm/lib/Transforms/Scalar/ComplexArithmetic.cpp:447
+  for (auto &I : *B) {
+    if (auto *SVI = dyn_cast<ShuffleVectorInst>(&I)) {
+      if (isInterleaving(SVI)) {
----------------
In my experience with complex instructions, one of the big issues is that the varied frontends end up creating a variety of patterns for complex arithmetic, so you end up with a random choice of a complex number being represented in LLVM IR as a struct, a vector, or pairs of scalar numbers (especially as the ABIs for passing or returning them through functions is so insane). That's leaving aside the question of how the complex multiply itself is represented (is it expanded in IR, or a call to `__mulsc3`, or both?).

By starting the search for a complex multiply at a shufflevector, you're really leaving a lot of opportunities to match complex multiplies off the table. The pattern-matching I did in D119288 looks for insertvalue, insertelement, and matching stores for things that might be the result of complex multiplies or divisions.

(This may also be because you're aiming to do this after vectorization, and one of the problems that I'm trying to solve with complex intrinsics is that the patterns of complex instructions generated by the frontend aren't always kosher for vectorization).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114174