[PATCH] D148558: [CodeGen] Improve handling -Ofast generated code by ComplexDeinterleaving pass

[Nicholas Guy via Phabricator via llvm-commits]

NickGuy added a comment.

Can you precommit the test changes in complex-deinterleaving-multiuses.ll?



================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:881
+      } else if (I->getOpcode() == Instruction::FSub) {
+        Worklist.emplace_back(I->getOperand(1), IsPositive ^ true);
+        Worklist.emplace_back(I->getOperand(0), IsPositive);
----------------
Nit: Don't think I've ever seen this way to negate a bool. It certainly works, but I'd argue that `!IsPositive` is a more conventional/readable way


================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:1069
+ComplexDeinterleavingGraph::NodePtr
+ComplexDeinterleavingGraph::identifyMultiplications(
+    std::list<Product> &RealMuls, std::list<Product> &ImagMuls,
----------------
One of the original design goals was to not assume full multiplications, instead representing them as 2 partial multiplys. Is that something that is possible to preserve while still accounting for `reassoc` and `-Ofast`?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:24710
+    if (Rotation == ComplexDeinterleavingRotation::Rotation_0) {
+      return B.CreateFAdd(InputA, InputB);
+    } else if (Rotation == ComplexDeinterleavingRotation::Rotation_180) {
----------------
Do we want to be discarding the fastmath flags here? 


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.h:23-24
 #include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/IR/CallingConv.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instruction.h"
----------------
Is this still necessary?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:22105
+    if (Rotation == ComplexDeinterleavingRotation::Rotation_0) {
+      return B.CreateFAdd(InputA, InputB);
+    } else if (Rotation == ComplexDeinterleavingRotation::Rotation_180) {
----------------
Do we want to be discarding the fastmath flags here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148558