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

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

In D116161#3211178 <https://reviews.llvm.org/D116161#3211178>, @junaire wrote:

> In order to use `emitUnaryBuiltin` in other cases, I changed the function interface.
> This allows us to use it in all `Builder.CreateUnaryIntrinsic()` cases, but will make
> the function body very small.

I think we should extend & use the existing `emitUnaryBuiltin`. I think in most cases where it cannot be used straight away you should be able to slightly rewrite the existing code to not rely on `llvm::Type`. Then there should be no need to call `EmitScalarExpr` early (an example is in the inline comments)



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:535
 // matching the argument type.
 static Value *emitUnaryBuiltin(CodeGenFunction &CGF,
                                const CallExpr *E,
----------------
I think we should extend this `emitUnaryBuiltin` function, rather than having a second one.

e.g.

```
 static Value *emitUnaryBuiltin(CodeGenFunction &CGF,
                                const CallExpr *E,
-                               unsigned IntrinsicID) {
+                               unsigned IntrinsicID, llvm::StringRef Name = "") {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));

   Function *F = CGF.CGM.getIntrinsic(IntrinsicID, Src0->getType());
-  return CGF.Builder.CreateCall(F, Src0);
+  return CGF.Builder.CreateCall(F, Src0, Name);
+}
```


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3190
     };
     Value *Op0 = EmitScalarExpr(E->getArg(0));
+    return RValue::get(emitUnaryBuiltin(
----------------
IICU the only reason to call `EmitScalarExpr` early is the use in `GetIntrinsicID`, but it could solely rely on `QualType`:


```
-    auto GetIntrinsicID = [](QualType QT, llvm::Type *IrTy) {
-      if (IrTy->isIntOrIntVectorTy()) {
-        if (auto *VecTy = QT->getAs<VectorType>())
-          QT = VecTy->getElementType();
-        if (QT->isSignedIntegerType())
-          return llvm::Intrinsic::vector_reduce_smax;
-        else
-          return llvm::Intrinsic::vector_reduce_umax;
-      }
+    auto GetIntrinsicID = [](QualType QT) {
+      if (auto *VecTy = QT->getAs<VectorType>())
+        QT = VecTy->getElementType();
+      if (QT->isSignedIntegerType())
+        return llvm::Intrinsic::vector_reduce_smax;
+      if (QT->isUnsignedIntegerType())
+        return llvm::Intrinsic::vector_reduce_umax;
+      assert(QT->isFloatingType() && "must have a float here");
       return llvm::Intrinsic::vector_reduce_fmax;
     };
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116161