[PATCH] Early expansion of MUX instructions on Hexagon

[Krzysztof Parzyszek]

Submitted in r233696.  Thanks all for comments!


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/Hexagon/HexagonExpandCondsets.cpp:257
@@ +256,3 @@
+/// going on, predicated instructions will have implicit uses of the re-
+/// gisters that are being defined. This is to keep any preceding defini-
+/// tions live. If there is no preceding definition, the implicit use will
----------------
qcolombet wrote:
> We usually do not use hyphens in comments.
Removed all word breaks from comments

================
Comment at: lib/Target/Hexagon/HexagonExpandCondsets.cpp:258
@@ +257,3 @@
+
+void HexagonExpandCondsets::makeDefined(unsigned Reg, SlotIndex S,
+      bool SetDef) {
----------------
qcolombet wrote:
> I think this function deserves a comment.
> From the name, it is not clear that this update the undef flag of the uses of Reg at S.
Will do.

================
Comment at: lib/Target/Hexagon/HexagonExpandCondsets.cpp:264
@@ +263,3 @@
+  assert(MI && "Expecting instruction");
+  for (MIOperands Mo(MI); Mo.isValid(); ++Mo) {
+    if (!Mo->isReg() || !Mo->isUse() || Mo->getReg() != Reg)
----------------
qcolombet wrote:
> You can use for-range based loop for that.
Will convert all such cases to range-based loops.

================
Comment at: lib/Target/Hexagon/HexagonExpandCondsets.cpp:299
@@ +298,3 @@
+        continue;
+      if (Mo->isDead())
+        Mo->setIsDead(false);
----------------
qcolombet wrote:
> This if is probably worth for performances than always setting the flag to false.
Will remove the if check.

================
Comment at: lib/Target/Hexagon/HexagonExpandCondsets.cpp:328
@@ +327,3 @@
+        continue;
+      if (Mo->isDead())
+        Mo->setIsDead(false);
----------------
qcolombet wrote:
> Ditto.
Same here.

================
Comment at: lib/Target/Hexagon/HexagonExpandCondsets.cpp:346
@@ +345,3 @@
+
+void HexagonExpandCondsets::clearKillFlags(MachineInstr *MI) {
+  for (MIOperands Mo(MI); Mo.isValid(); ++Mo)
----------------
qcolombet wrote:
> This functionality is already available via MachineInstr::clearKillInfo().
Will do.

================
Comment at: lib/Target/Hexagon/HexagonExpandCondsets.cpp:353
@@ +352,3 @@
+
+void HexagonExpandCondsets::resetKillFlags(unsigned Reg, LiveInterval &LI) {
+  MRI->clearKillFlags(Reg);
----------------
qcolombet wrote:
> This method would deserve a comment as well and/or a renaming.
> 
> => You mark all the uses as kill.
I changed the name to updateKillFlags and added comment explaining the functionality.

================
Comment at: lib/Target/Hexagon/HexagonExpandCondsets.cpp:527
@@ +526,3 @@
+      Ko->setIsKill(false);
+      // I suppose there should be only one operand with <kill> for UseR.
+      break;
----------------
qcolombet wrote:
> Not necessarily.
> 
> Anyway, use MachineInstr::clearRegisterKills.
Will use clearRegisterKills.

http://reviews.llvm.org/D8329

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/