[PATCH] D114075: [IR] Split vscale_range interface

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes marked an inline comment as done.
c-rhodes added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1611-1615
+      Optional<unsigned> MaxVScale =
+          CI.getFunction()
+              ->getFnAttribute(Attribute::VScaleRange)
+              .getVScaleRangeMax();
+      if (MaxVScale && Log2_32(MaxVScale.getValue()) < (SrcBitSize - 1)) {
----------------
sdesmalen wrote:
> nit: this is probably a stylistic thing, but I personally find the following a bit nicer to read:
> 
>   Attribute Attr = CI.getFunction()->getFnAttribute(Attribute::VScaleRange);
>   if (Optional<unsigned> MaxVScale = Attr.getVScaleRangeMax())
>     if (Log2_32(MaxVScale.getValue()) < (SrcBitSize - 1)) {
>       ..
>     }
> 
> (same suggestion for the similar cases)
> nit: this is probably a stylistic thing, but I personally find the following a bit nicer to read:
> 
>   Attribute Attr = CI.getFunction()->getFnAttribute(Attribute::VScaleRange);
>   if (Optional<unsigned> MaxVScale = Attr.getVScaleRangeMax())
>     if (Log2_32(MaxVScale.getValue()) < (SrcBitSize - 1)) {
>       ..
>     }
> 
> (same suggestion for the similar cases)

I agree, done


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

https://reviews.llvm.org/D114075