[PATCH] D111434: [PowerPC] PPC backend optimization on conditional trap intrustions

[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. There are some very minor nits that can be addressed on the commit.



================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1018
+        unsigned Opcode2 = LiMI2->getOpcode();
+        bool isOperand2Immeidate = MI.getOperand(2).isImm();
+        // We can only do the optimization for the "reg + reg" form.
----------------
amyk wrote:
> 
Nit: naming convention - variables start with uppercase letters.


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1019
+        bool isOperand2Immeidate = MI.getOperand(2).isImm();
+        // We can only do the optimization for the "reg + reg" form.
+        if (!(LiMI1 && (Opcode1 == PPC::LI || Opcode1 == PPC::LI8)))
----------------
I don't understand this comment. You say we can only do the optimization for the "reg + reg" form but the second condition is actually for the "reg + imm" form.


================
Comment at: llvm/test/CodeGen/PowerPC/mi-peepholes-trap-opt.mir:5
+---
+name:            conditional_trap_opt_reg_implicit_def
+alignment:       16
----------------
Please add a couple more tests:
1. Test case where we delete the instruction because it won't trap
2. Test case(s) where we do some combination of comparisons (NE(24): `<>`, LE(20): `<=`, etc.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111434