[PATCH] D136793: [AArch64][GlobalISel] Add some minor post-selection optimizations.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp:89
+      continue; // We may have erased this instruction.
+    Changed |= setKillFlags(MI);
+  }
----------------
arsenm wrote:
> I'd be disappointed if this is actually helping. Do you have examples where kill flags are needed that aren't added by LiveVariables?
Huh, you're right. I'm not sure why I thought this was making an improvement, must have been measuring the wrong change. I'll remove this from the patch.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp:108
+
+  if (!Src.isVirtual() || !Dst.isVirtual())
+    return false;
----------------
paquette wrote:
> `isPhysical`?
Registers can be either physical, virtual or stack slots apparently, so it's better to use `isVirtual()`.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp:132
+    // This is the inverse case, where the destination class is a superclass of
+    // the source. Here, if the copy is the only user, we can just constrain
+    // the user of the copy to use the smaller class of the source.
----------------
paquette wrote:
> We need the one-use check here right?
No, if the COPY source register is a subclass of the destination, we can just replace all uses of the dest with the source safely, so we can do it for every use of the COPY's dst.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp:150
+
+  for (unsigned Idx = 0; Idx < MI.getNumExplicitOperands(); ++Idx) {
+    auto &Op = MI.getOperand(Idx);
----------------
paquette wrote:
> I think you can just use `explicit_operands()` here?
I need `Idx` for the `getOperandConstraint()` call below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136793