[PATCH] D101062: [AArch64][SVE] Better utilisation of immediate forms for bitwise intrinsics
Bradley Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 27 03:57:04 PDT 2021
bsmith added inline comments.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:4994
+ SDValue &Op = N;
+ while (Op.getOpcode() == AArch64ISD::REINTERPRET_CAST) {
+ Op = Op.getOperand(0);
----------------
sdesmalen wrote:
> I had expected there to be only a single REINTERPRET_CAST when it gets to the instruction selection phase, but I guess there are no combines that remove successive REINTERPRET_CASTs at the moment?
I'm not actually sure if there will always only be a single cast or not, but it feels like it's probably safer to just assume there isn't as it doesn't really add any complexity.
================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:354
+class SVE_1_Op_Imm_Log_Pred_Pat<ValueType vt, ValueType pt, SDPatternOperator op,
+ ZPRRegOp zprty, ValueType it, ComplexPattern cpx, Instruction inst>
----------------
sdesmalen wrote:
> You're probably just following precedent, but I find the naming here a bit confusing since I'd think `_Pat` suggests the pattern takes a predicate. In this case, my preference would be to pass the `(SVEAllActive)` as operand/patfrag to the pattern class.
Perhaps changing this to be `SVE_1_Op_Pred_Imm_Log_Pat` would be better? (And changing the equivalent `Arith` version above). Passing in `(SVEAllActive)` in as a parameter just seems like it's adding parameters for the sake of it as nothing needs to pass this in differently.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101062/new/
https://reviews.llvm.org/D101062
More information about the llvm-commits
mailing list