[PATCH] D117689: [AArch64][SVE] Fold vselect into predicated fmul, fsub and fadd
Peter Waller via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 26 08:32:57 PST 2022
peterwaller-arm added inline comments.
================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:8468
+ ]>;
\ No newline at end of file
----------------
I don't think this should mimic the form of the other (SVE_...) names, since those are for defining patterns, whereas this is a `PatFrags`, which is a different kind of thing. So in my view the name should be distinct to try and avoid catching unsuspecting readers into another trap.
How about `EitherVSelectOrPassthruPatFrags`. I'm thinking along the lines of trying to avoid the error of `Or` reading as the binary operation by prefacing it with `Either`.
Also, this doesn't look like the right place for this. I think a better location would be up near this, prehaps right at the end of that block before the `getSVEPseudoMap` definition. For good measure I might put a comment-block delimiter which separates it from the patterns above, and indicate in the comment that this "matches either an intrinsic, or a predicated operation with an all active predicate".
```
//===----------------------------------------------------------------------===//
// SVE pattern match helpers.
//===----------------------------------------------------------------------===//
```
Note also the missing trailing newline phabricator is complaining about.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117689/new/
https://reviews.llvm.org/D117689
More information about the llvm-commits
mailing list