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

[Allen zhong via Phabricator via llvm-commits]

Allen 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))
----------------
efriedma wrote:
> 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.
Ok, I add your comment., thanks


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:281
+  // conservatively.
+  if (!SrcMI || (SrcMI->getOpcode() == TargetOpcode::COPY) ||
+      (SrcMI->getOpcode() <= TargetOpcode::GENERIC_OP_END) ||
----------------
efriedma wrote:
> Checking "!SrcMI" twice.
Thanks, delete the duplicate checking "!SrcMI" 


================
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) ||
----------------
efriedma wrote:
> `TargetOpcode::COPY <= TargetOpcode::GENERIC_OP_END` should be true.
Thanks, apply your comment


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:284
+      !TRI->isTypeLegalForClass(*RC, MVT::i64) ||
+      TRI->isTypeLegalForClass(*RC, MVT::f64))
+    return false;
----------------
efriedma wrote:
> 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?
I add the isTypeLegalForClass checks because this changes will cause many test cases to be updated, especially some vector type register class.
```
def FPR64 : RegisterClass<"AArch64", [f64, i64, v2f32, v1f64, v8i8, v4i16, v2i32,
                                      v1i64, v4f16, v4bf16],
                                     64, (sequence "D%u", 0, 31)>;
```
Now I updated with **AArch64::GPR64allRegClass.hasSubClassEq(RC)** as your comment to check integer registers, thanks!


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

https://reviews.llvm.org/D132939