These kind of go together, since fcmp expansion generates boolean operations.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
7563 | It seems like this code is missing a guard for VT.isScalableVector() (same for the aarch64_sve_whilelo case below) | |
llvm/lib/Target/AArch64/SVEInstrFormats.td | ||
1374 | Do we want to create a SVE_2_Op_<..>_Pat or something for these? There will be other instructions where this would be useful as well. nit: please reverse the order so it matches the patterns straight above it. |
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
7563 | Not sure what you think needs to be handled; non-scalable i1 vectors aren't legal. | |
llvm/lib/Target/AArch64/SVEInstrFormats.td | ||
1374 | I'm not sure what that would look like. Are you proposing something like SVE_2_Op_PTRUE_H_Pat/SVE_2_Op_PTRUE_S_Pat/etc.? |
llvm/lib/Target/AArch64/SVEInstrFormats.td | ||
---|---|---|
1374 | There's something similar in D71712: class SVE_2_Op_AllActive_Pat<ValueType vtd, SDPatternOperator op, ValueType vt1, ValueType vt2, Instruction inst, Instruction ptrue> : Pat<(vtd (op vt1:$Op1, vt2:$Op2)), (inst (ptrue 31), $Op1, $Op2)>; Seems like a good fit, but I'm not sure... |
llvm/lib/Target/AArch64/SVEInstrFormats.td | ||
---|---|---|
1374 | So this would be written like the following? def : SVE_2_Op_AllActive_Pat<nxv2i1, op_nopred, nxv2i1, nxv2i1, !cast<Instruction>(NAME), PTRUE_D>; It doesn't seem much better than what I've written, but okay, I guess. |
LGTM!
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
7563 | Yes, of course! Thanks for adding that comment. | |
llvm/lib/Target/AArch64/SVEInstrFormats.td | ||
1374 | Cheers, FWIW this makes it a bit more readable to me (especially in conjunction with the use of SVE_3_Op_Pat above). | |
1376 | This file doesn't seem too strict on the 80char limit, so I think having this on the same line would make it more readable. |
llvm/lib/Target/AArch64/SVEInstrFormats.td | ||
---|---|---|
1376 | It's not strict, yes, but I think going over 100 columns is probably not a good idea. |
It seems like this code is missing a guard for VT.isScalableVector() (same for the aarch64_sve_whilelo case below)