[PATCH] D132939: [Peephole] rewrite INSERT_SUBREG to SUBREG_TO_REG if upper bits zero

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:261
+  // From %reg = INSERT_SUBREG %reg, %subreg, subidx
+  // To   %reg:subidx =  SUBREG_TO_REG 0, %subreg, subidx
+  if (!MI.isRegTiedToDefOperand(1))
----------------
If I'm understanding correctly, we're assuming the first operand to INSERT_SUBREG is irrelevant because a COPY would destroy the upper part of the register anyway?  Maybe makes sense to explicitly note that.


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:281
+  // conservatively.
+  if (!SrcMI || (SrcMI->getOpcode() == TargetOpcode::COPY) ||
+      (SrcMI->getOpcode() <= TargetOpcode::GENERIC_OP_END) ||
----------------
Checking "!SrcMI" twice.


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:282
+  if (!SrcMI || (SrcMI->getOpcode() == TargetOpcode::COPY) ||
+      (SrcMI->getOpcode() <= TargetOpcode::GENERIC_OP_END) ||
+      !TRI->isTypeLegalForClass(*RC, MVT::i64) ||
----------------
`TargetOpcode::COPY <= TargetOpcode::GENERIC_OP_END` should be true.


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:284
+      !TRI->isTypeLegalForClass(*RC, MVT::i64) ||
+      TRI->isTypeLegalForClass(*RC, MVT::f64))
+    return false;
----------------
What are you trying to accomplish with these isTypeLegalForClass checks?  You're looking for a register class that only contains integer registers?  Can you just check that the register class is a subclass of GPR64all?


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

https://reviews.llvm.org/D132939