[PATCH] D111403: [Inliner] Don't inline functions with incompatible vscale_range attributes

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1800
+  // don't match.
+  if (!CallerMinVScale || !CallerMaxVScale || !CalleeMinVScale ||
+      !CalleeMaxVScale)
----------------
sdesmalen wrote:
> Currently this code would avoid inlining a function with
> 
>   `vscale_range(0, 16)` into a function with `vscale_range(4, 16)`
> 
> which we should probably allow.
> 
> For the 'unbounded' minimum I think we can interpret this as '1' (i.e. the minimally required vscale could be anything, it means it (also) has to run on a vscale of 1, so we can assume 1).
> For the unbounded maximum we can interpret this as UINT_MAX (i.e. the length of the vector is unknown, so assume worst-case)
> 
> And then do the individual comparisons on those newly interpreted values.
I wonder what the point is of having a 0 for the minimum here then? It feels like we should just never allow a 0 minimum for vscale_range as I don't think the LangRef is particularly clear. The way I read the LangRef it sounds like min could be anything, like 8 or 3. If min could be >4 then it's not safe to inline, right? At the moment it feels like we're tightening the meaning of the vscale_range attribute without changing the documentation, which feels a bit dangerous that's all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111403/new/

https://reviews.llvm.org/D111403