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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
