[PATCH] D114075: [IR] Split vscale_range interface

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/include/llvm/IR/Attributes.h:224
+  /// Returns the optional maximum value for the vscale_range attribute.
+  Optional<unsigned> getVScaleRangeMax() const;
 
----------------
paulwalker-arm wrote:
> bsmith wrote:
> > I'm not sure how I feel about this being an Optional type, all of the code changes below are still having to check `max` against `0`, it seems that having this be Optional is just complicating the code for no gain.
> > 
> > Either this should not be Optional, or a value of 0 should be disallowed for max from this interface.
> I agree.  I believe the intent is for `getVScaleRangeMax` to only return a real >0 value or None otherwise.
thanks for comments and sorry for only just getting back to you, I've updated the patch so 0 is replaced with None in the interface, hopefully this makes more sense now.


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

https://reviews.llvm.org/D114075