junaire added a comment. In D116161#3209292 <https://reviews.llvm.org/D116161#3209292>, @fhahn wrote:
> In D116161#3209286 <https://reviews.llvm.org/D116161#3209286>, @junaire wrote: > >> 35: %0 = load float, float* %f1.addr, align 4 >> 36: %1 = load float, float* %f1.addr, align 4 >> 37: %elt.abs = call float @llvm.fabs.f32(float %1) > > It looks like the argument expression is evaluated twice. Did you remove the > ` Value *Op0 = EmitScalarExpr(E->getArg(0));` calls? Well, for example: For `__builtin_elementwise_abs`, we have code like: case Builtin::BI__builtin_elementwise_abs: { Value *Op0 = EmitScalarExpr(E->getArg(0)); Value *Result; if (Op0->getType()->isIntOrIntVectorTy()) Result = Builder.CreateBinaryIntrinsic( llvm::Intrinsic::abs, Op0, Builder.getFalse(), nullptr, "elt.abs"); else Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::fabs, Op0, nullptr, "elt.abs"); return RValue::get(Result); } If we use `emitUnaryBuiltin`, we are supposed to do something like: Result = emitUnaryBuiltin(*this, E, llvm::Intrinsic::fabs, "elt.abs"); We need to pass `CallExpr* E` to the function to meet its interface, and it calls `CGF.EmitScalarExpr(E->getArg(0));` inside. Maybe this is what you said about the argument expression being evaluated twice? I think it is avoidable if we don't change the function's interface. Maybe we can have something like: static Value *emitUnaryBuiltin(CodeGenFunction &CGF, Value* Op0, unsigned IntrinsicID, llvm::StringRef Name) { return CGF.Builder.CreateUnaryIntrinsic(IntrinsicID, Op0, nullptr, Name); } Then for `__builtin_elementwise_abs` we can have: Result = emitUnaryBuiltin(*this, Op0, llvm::Intrinsic::fabs, "elt.abs"); and for `__builtin_elementwise_ceil` have: return RValue::get( emitUnaryBuiltin(*this, EmitScalarExpr(E->getArg(0)), llvm::Intrinsic::ceil, "elt.ceil")); WDYT? Well, franking speaking I think this one-line function is ugly but I can't come up with a more elegant solution, I would appreciate it if you can offer some suggestions. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116161/new/ https://reviews.llvm.org/D116161 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
