[PATCH] D114174: [ARM][CodeGen] Add support for complex deinterleaving

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/include/llvm/InitializePasses.h:96
 void initializeCFGSimplifyPassPass(PassRegistry&);
-void initializeCFGuardPass(PassRegistry&);
-void initializeCFGuardLongjmpPass(PassRegistry&);
-void initializeCFGViewerLegacyPassPass(PassRegistry&);
-void initializeCFIFixupPass(PassRegistry&);
-void initializeCFIInstrInserterPass(PassRegistry&);
-void initializeCFLAndersAAWrapperPassPass(PassRegistry&);
-void initializeCFLSteensAAWrapperPassPass(PassRegistry&);
-void initializeCallGraphDOTPrinterPass(PassRegistry&);
-void initializeCallGraphPrinterLegacyPassPass(PassRegistry&);
-void initializeCallGraphViewerPass(PassRegistry&);
-void initializeCallGraphWrapperPassPass(PassRegistry&);
-void initializeCallSiteSplittingLegacyPassPass(PassRegistry&);
+void initializeCFGuardPass(PassRegistry &);
+void initializeCFGuardLongjmpPass(PassRegistry &);
----------------
You can undo these


================
Comment at: llvm/include/llvm/InitializePasses.h:115
 void initializeConstraintEliminationPass(PassRegistry &);
-void initializeCorrelatedValuePropagationPass(PassRegistry&);
-void initializeCostModelAnalysisPass(PassRegistry&);
-void initializeCrossDSOCFIPass(PassRegistry&);
+
+void initializeCorrelatedValuePropagationPass(PassRegistry &);
----------------
There is a newline here?


================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:412
+  if (!Real->hasOneUse() || !Imag->hasOneUse()) {
+    LLVM_DEBUG(dbgs() << "  - Oneuse\n");
+    return nullptr;
----------------
If you can turn some of these into better sentances that would be good. Perhaps "Mul instruction has multiple uses" like you have below.


================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:481
+  UncommonImagOp = (CommonOperand == I0) ? I1 : I0;
+  if (Rotation == ComplexDeinterleavingRotation::Rotation_90 || Rotation == ComplexDeinterleavingRotation::Rotation_270)
+    std::swap(UncommonRealOp, UncommonImagOp);
----------------
Formatting is a bit long.


================
Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:437
   // Match interleaved memory accesses to ldN/stN intrinsics.
-  if (TM->getOptLevel() != CodeGenOpt::None)
+  if (TM->getOptLevel() != CodeGenOpt::None) {
     addPass(createInterleavedAccessPass());
----------------
dmgreen wrote:
> This doesn't need to add the new brackets.
This again.


================
Comment at: llvm/test/CodeGen/ARM/O3-pipeline.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=arm -O3 -debug-pass=Structure < %s -o /dev/null 2>&1 | grep -v "Verify generated machine code" | FileCheck %s
----------------
This looks like it was incorrectly added for this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114174