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