frasercrmck added a comment. In D107290#3268949 <https://reviews.llvm.org/D107290#3268949>, @paulwalker-arm wrote:
> Does this mean `RISCVTTIImpl::getMaxVScale()` can be removed? Good question. I'm unsure at this stage. At hinted at in the description, `getMaxVScale` can make use of backend-specific flags to hone the maximum down a bit, whereas relying on the attribute basically reduces us to the one value which the frontend will ever likely produce. So as it stands, the `vscale_range` attribute is not at feature parity with this TTI method. I think we'd have to come to a decision that this outcome is okay. ================ Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vscale-range.ll:162 + +attributes #0 = { vscale_range(2,1024) } +attributes #1 = { vscale_range(4,1024) } ---------------- khchen wrote: > frasercrmck wrote: > > khchen wrote: > > > I'm thinking do we need to test zvl and vscale_range in the same > > > attribute? > > > ex. `attributes #0 = { vscale_range(2,1024) "target-features"="+zvl512b" > > > }` > > Perhaps yeah. Just to check - what exactly for? Because we need `zvl` in > > the attributes for correctness, or in order to test the combination of > > `zvl` architecture and `vscale_range` to test what happens when they > > disagree? > Just test for they disagree. > Do you know what's expected value for different `vscale_range` value in two > function after function inlining? If they are always have the same minimum > value for VLEN, I think we don't need a check. Good idea. As for inlining, I can't see anything that would //prevent// inlining of functions with different `vscale_range` attributes, per se. However, I was looking at `TTI::areInlineCompatible` and the default implementation checks whether CPU/Feature Strings are equivalent. The frontend should ensure that `vscale_range` attributes match up 1:1 with our `+zvl` feature strings so I think in practice we won't inline functions with different `zvl` values in clang-generated C/C++ code. But users could write IR with different `vscale_range` attributes and we'd happily inline them, which sounds fishy. What do you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107290/new/ https://reviews.llvm.org/D107290 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits