[PATCH] D116161: [Clang] Add an overload for emitUnaryBuiltin.

Jun Zhang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 24 07:10:56 PST 2021

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");
      Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::fabs, Op0, nullptr,
    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. :)

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list