[PATCH] D112285: [PowerPC] PPC backend optimization to lower int_ppc_tdw/int_ppc_tw intrinsics to TDI/TWI machine instructions

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM other than a number of stylistic changes. Feel free to address those on the commit. You also might want to give @amyk a bit of time to ensure her comments were adequately addressed.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5017
+        Opcode = IntrinsicID == Intrinsic::ppc_tdw ? PPC::TD : PPC::TW;
+      }
+      // We will emit PPC::TDI or PPC::TWI if the 2nd and 3rd operands are reg +
----------------
Nit: no braces for a single statement.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5020
+      // imm or imm + reg.
+      else {
+        Opcode = IntrinsicID == Intrinsic::ppc_tdw ? PPC::TDI : PPC::TWI;
----------------
Nit: keep the `else` on the line immediately following the end of the `if` block so it is visually easy to match them up (the comments for `else/else if` go into the block). Also, rather than the structure:
```
if (something) {
} else {
  if (something else)
    ...
  else
    ...
}
```
Opt for a more flat structure of
```
if (something)
else if (something else)
else
```


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5021
+      else {
+        Opcode = IntrinsicID == Intrinsic::ppc_tdw ? PPC::TDI : PPC::TWI;
+        // The 2nd and 3rd operands are reg + imm.
----------------
If you initialize `Opcode` this way at the declaration, you won't need this here and can flatten this as per my above comment.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5025
+          Ops[2] = getI32Imm(int(SImmOperand3) & 0xFFFF, dl);
+        }
+        // The 2nd and 3rd operands are imm + reg.
----------------
Nit: no braces with a single statement.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5037
+            TO = (TO & 0x1) ? TO + 1 : TO - 1;
+          // We swap the fourth and fifthy bit of TO if they are not same.
+          if ((TO & 0x8) != ((TO & 0x10) >> 1))
----------------
s/fifthy/fifth


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112285