david-arm added inline comments.
================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:671
+ case AArch64SVEPredPattern::vl8:
+ return NumElts >= Pattern
+ ? Optional<Instruction *>(IC.replaceInstUsesWith(
----------------
I was actually wondering if we could commonise this code somehow. Perhaps by
setting a MinNumElts variable in the case statements, i.e.
unsigned MinNumElts;
...
case AArch64SVEPredPattern::vl8:
MinNumElts = Pattern;
break;
case AArch64SVEPredPattern::vl16:
MinNumElts = 16;
break;
}
if (NumElts < MinNumElts) return None;
return Optional<Instruction *>(IC.replaceInstUsesWith(
II, ConstantInt::get(II.getType(), NumElts)));
================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:662
+ return IC.replaceInstUsesWith(II, StepVal);
+ } else if (Pattern == AArch64SVEPredPattern::vl16 && NumElts == 16) {
+ Constant *StepVal = ConstantInt::get(II.getType(), NumElts);
----------------
junparser wrote:
> david-arm wrote:
> > Could you potentially fold these two cases into one somehow? Maybe with a
> > switch-case statement? I'm just imagining a situation where we might want
> > other patterns too like vl32, vl64, etc.
> >
> There is no other special pattern except vl16. But I do think switch-case is
> more straightforward
OK, thanks for making this a switch statement. I was just thinking that in the
developer manual we say we also support vl1-vl256 so at some point we may add
more enums in LLVM too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104852/new/
https://reviews.llvm.org/D104852
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
