[PATCH] D70181: [MVE] [ARM] Select VQABS

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added a comment.

Thanks, this does look much nicer!

Now I can read it more easily, I can see that what you're actually matching is effectively the vectorization of a pair of nested ternary-expressions, so that each lane is being independently transformed via the function

  x > 0 ? x 
        : (x == INT_MIN ? INT_MAX
                        : -x)

which it's reasonably clear does implement the same function as the `vqabs` instruction.

I only have one small nitpick left.



================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:1962
+    def : Pat<(VTI.Vec (vselect
+                      (VTI.Pred (ARMvcmpz (VTI.Vec MQPR:$reg), (i32 12))),
+                      (VTI.Vec MQPR:$reg),
----------------
This `(i32 12)`, and the `(i32 0)` in the `ARMvcmp` a couple of lines below, are the only remaining magic numbers that have no explanation here. I think they deserve a comment explaining them: 12 and 0 are respectively the Arm architecture's condition-code encodings for GT and EQ, so this `ARMvcmpz` is testing if each lane is //greater than// zero, and the `ARMvcmp` below is testing if each lane is //equal to// (the corresponding lane of) `int_min`.


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

https://reviews.llvm.org/D70181