[PATCH] D84587: [ConstantRange] Add API for intrinsics (NFC)

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 27 03:19:00 PDT 2020


fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, this makes sense to enable wider use and is in line with existing members like `binaryOp`. Thanks!



================
Comment at: llvm/include/llvm/IR/ConstantRange.h:153
 
+  /// Whether ConstantRange calculations for this intrinsic are supported.
+  static bool isIntrinsicSupported(Intrinsic::ID IntrinsicID);
----------------
Nit: mentioning explicitly what is returned seems a bit clearer to me, .e.g `Returns true if ConstantRange calculations are supported for intrinsic with \p IntrinsicID`.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:863
+  default:
+    llvm_unreachable("Unsupported intrinsic");
+  }
----------------
maybe we should `assert(!isIntrinsicSupported(IntrinsicID))` here, to catch (some) inconsistencies between `isIntrinsicSupported` and `intrinsic`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84587





More information about the llvm-commits mailing list