[PATCH] D30431: [PowerPC] MachineSSA pass to reduce the number of CR-logical operations

[Tony Jiang via Phabricator via llvm-commits]

jtony added inline comments.


================
Comment at: lib/Target/PowerPC/PPCMachineBasicBlockUtils.h:93
+/// MachineBranchProbabilityInfo isn't null.
+static bool splitMBB(MachineInstr *OrigBranch, MachineInstr *SplitBefore,
+                     MachineInstr *SplitCond, bool InvertNewBranch,
----------------
It looks to me this static function uses too many parameters, which makes it a little bit hard to understand. Is it possible for us to reduce the number of parameters used here? One way I could think of is to make this a member function of class PPCReduceCRLogicals. In that case we could make some of the parameters member of the class, so that we don't have to pass them around. But it is totally up to you to decide whether the effort is worthwhile or not.


================
Comment at: lib/Target/PowerPC/PPCReduceCRLogicals.cpp:220
+
+    // FIXME: remove the variable, just return the result of simplifyCode()
+    bool Changed = false;
----------------
Can we do what the `FIXME` suggested here in this patch?  Since the community doesn't like us putting `FIXME` in the code.


Repository:
  rL LLVM

https://reviews.llvm.org/D30431