paulwalker-arm added inline comments.
================ Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:119-131 -Optional<unsigned> RISCVTTIImpl::getMaxVScale() const { - // There is no assumption of the maximum vector length in V specification. - // We use the value specified by users as the maximum vector length. - // This function will use the assumed maximum vector length to get the - // maximum vscale for LoopVectorizer. - // If users do not specify the maximum vector length, we have no way to - // know whether the LoopVectorizer is safe to do or not. ---------------- bsmith wrote: > I'm not sure that RISCV have made a commitment to use the vscale_range > attribute yet have they? In either case I think they should be involved in a > change like this. Perhaps it's worth this patch not removing getMaxVScale just yet but rather just AArch64's implementation? There would only need to be a minor change to LoopVectorize.cpp along the lines of `if (!MaxVScale && TheFunction->hasFnAttribute(Attribute::VScaleRange...`. That way getMaxVScale can be removed if/when no one needs it, which I hope is not too far away. If there is agreement to remove it then I imagine code similar to what you've done for SVE in CodeGenFunction.cpp will be needed for RISCV otherwise the patch will cause a regression in functionality. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106277/new/ https://reviews.llvm.org/D106277 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits