[PATCH] D151492: Add fastmath attributes to llvm.call_intrinsic

[Lily Orth-Smith via Phabricator via cfe-commits]

electriclilies added inline comments.


================
Comment at: mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp:98
 static LogicalResult
 convertCallLLVMIntrinsicOp(CallIntrinsicOp &op, llvm::IRBuilderBase &builder,
                            LLVM::ModuleTranslation &moduleTranslation) {
----------------
electriclilies wrote:
> Mogball wrote:
> > Mogball wrote:
> > > electriclilies wrote:
> > > > Mogball wrote:
> > > > > Mogball wrote:
> > > > > > electriclilies wrote:
> > > > > > > Mogball wrote:
> > > > > > > > I think the problem with the cast below is that this is passed by reference. You should not pass operation handles by reference.
> > > > > > > I think I can't pass it by value because it's const though.. 
> > > > > > where is it const?
> > > > > if it were const, it wouldn't bind to a non-const reference anyways
> > > > If I just try to pass the op into the dyn_cast, it gives this error, which says that the op is a const
> > > > ```
> > > > /home/lily/modular/third-party/llvm-project/llvm/include/llvm/Support/Casting.h:64:65: error: cannot initialize a parameter of type 'mlir::Operation *' with an rvalue of type 'const mlir::LLVM::CallIntrinsicOp *
> > > > ```
> > > did you drop the reference?
> > ```
> > CallIntrinsicOp &op
> > ```
> > 
> > delete the `&`. You should never do this for ops anyways
> OK, I think I can upcast it to an Operation& to avoid this, and get rid of the copy
Dropped the reference, it works now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151492