[PATCH] D71888: [mlir] Added llvm.resume and personality functions in LLVM IR Dialect

[shraiysh via Phabricator via llvm-commits]

shraiysh added inline comments.


================
Comment at: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp:637
+  for (Value v : values) {
+    assert(valueMapping.find(v) != valueMapping.end() &&
+           "referencing undefined value");
----------------
rriddle wrote:
> mehdi_amini wrote:
> > shraiysh wrote:
> > > bondhugula wrote:
> > > > shraiysh wrote:
> > > > > rriddle wrote:
> > > > > > nit: llvm::is_contained
> > > > > I think `llvm::is_contained` is a wrapper for `std::find`. I am not sure if there is a neat way of looking for keys in a map using `std::find` (and hence `llvm::is_contained`). Please let me know if there is.
> > > > It might be a wrapper around it, but it's more readable to me with llvm::is_contained. 
> > > I meant `llvm::is_contained(valueMapping, v)` does not work - because `valueMapping` is a `DenseMap`. I am not aware of a variant of `is_contained` that supports maps - looking for a key in it. Currently I am using `std::map::find` while the documentation suggests `is_contained` is wrapper for `std::find`.
> > rather `valueMapping.count(v)` ?
> Gah, I was hoping that llvm::is_contained would be smart enough to detect find at this point. Given that, you should use .count() instead.
Done!


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

https://reviews.llvm.org/D71888