[PATCH] D108793: Teach the AArch64 backend patterns to generate the EOR3 instruction.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:974
+class EOR3_pattern<ValueType VecTy>
+  : Pat<(xor (xor (VecTy V128:$Vn), (VecTy V128:$Vm)), (VecTy V128:$Va)),
+        (EOR3 (VecTy V128:$Vn), (VecTy V128:$Vm), (VecTy V128:$Va))>;
----------------
resistor wrote:
> craig.topper wrote:
> > I don't the AArch64 ISA, but is it possible one of the xors is a vnot. And if that is possible, is EOR3 what you want to generate or would you want XOR+NOT?
> This still behaves correctly with this patch. XOR + VNOT generates EOR + MVN.
Sounds like it is worth adding some quick tests.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:976
+        (EOR3 (VecTy V128:$Vn), (VecTy V128:$Vm), (VecTy V128:$Va))>;
+  def : Pat<(xor (VecTy V128:$Vn) (xor (VecTy V128:$Vm), (VecTy V128:$Va)),
+        (EOR3 (VecTy V128:$Vn), (VecTy V128:$Vm), (VecTy V128:$Va))>;
----------------
resistor wrote:
> dmgreen wrote:
> > dmgreen wrote:
> > > There's a comma missing here. The brackets also look off.
> > Oh xor is also commutative. You likely don't need both the patterns, it will be handled automatically by tablegen.
> > 
> > Having tests that check that sound good to have though.
> I do have tests for the different ways to commute it already. Good call that tblgen should handle the commutation automatically.
I don't believe those tests are actually commutative. %4 is always on the right hand side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108793