[PATCH] D109850: Implement SystemZIselLowering::hasAndNot

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added inline comments.


================
Comment at: llvm/lib/Target/SystemZ/SystemZInstrVector.td:933
+    def : Pat<(type (xor (and (xor VR128:$x, (z_vneg VR128:$x)), (z_vsra_by_scalar VR128:$x, (i32 shift))), VR128:$x)),
+              (lc (lp VR128:$x))>;
   }
----------------
Are you sure these are right?  Unless I'm missing something, they appear to be swapped:

Looking at the first pattern, if `x` is positive then the shift value will be 0, therefore the `and` will be 0, therefore the `xor` will be a no-op and the return is `-x`.   If `x` is negative, the shift value will be -1, so the `and` is a no-op, and the result is `-x ^ x ^ -x`, which is `x`.   So in summary the pattern matches `x >= 0 ? -x : x`, which is the *negated* absolute value, while the pattern resolves to `lp`, which compute the absolute value.

Also, I think these patterns would be more readable if broken up into multiple lines like the other patterns in this group - the indentation shows more easily how the operations are grouped.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109850