[PATCH] D48832: [ARM] ARMCodeGenPrepare backend pass

[John Brawn via Phabricator via llvm-commits]

john.brawn added inline comments.


================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:89
+  SmallPtrSet<Value*, 16> CurrentVisited;
+  SmallVector<Instruction*, 8> InstsToRemove;
+  Module *M = nullptr;
----------------
This is unused.


================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:519
+
+  OrigTy = V->getType();
+  TypeSize = OrigTy->getPrimitiveSizeInBits();
----------------
OrigTy is only used in this function, so should be a local variable.


================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:531
+  WorkList.insert(V);
+  CurrentVisited.clear();
+
----------------
CurrentVisited is only used in this function, and cleared before being used, so it would make more sense as a local variable.


================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:617
+bool ARMCodeGenPrepare::doInitialization(Module &M) {
+  this->M = &M;
+  ExtTy = Type::getInt32Ty(M.getContext());
----------------
M isn't used outside of this function, so you can get rid of it.


================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:618
+  this->M = &M;
+  ExtTy = Type::getInt32Ty(M.getContext());
+  Promoter = new IRPromoter(&M);
----------------
ExtTy is never changed and only passed to IRPromoter::Mutate, so it looks like it would make more sense for it to be just a local variable in IRPRomoter::Mutate.


================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:627
+
+  auto *TPC = getAnalysisIfAvailable<TargetPassConfig>();
+  if (!TPC)
----------------
Why IfAvailable? Why not define ARMCodeGenPrepare::getAnalysisUsage and require it there so it's always available?


================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:631
+
+  F = &f;
+  const TargetMachine &TM = TPC->getTM<TargetMachine>();
----------------
As far as I can tell F isn't used outside of this function, so you can get rid of it.


https://reviews.llvm.org/D48832