[clang] Move int <-> float scalar conversion to a separate function

[SaurabhJha]

Summary

As prelude to this patch https://reviews.llvm.org/D99037, we want to move the int-float conversion
into a separate function that can be reused by matrix cast

[fhahn]

clang/lib/CodeGen/CGExprScalar.cpp

1209

now that' this is a separate function, we should be able to just return the created value directly, right? There should be no need for Res

[erichkeane]

clang/lib/CodeGen/CGExprScalar.cpp

1209

Yep, I'd also prefer multiple returns. That also cleans up a bunch of hte 'else's in this function.

[erichkeane]

Hrmph... Phab ate my other comment, which was that the name EmitCastBetweenScalarTypes feels clunky. Does EmitScalarCast or EmitScalarScalarCast sound better and capture the meaning correctly?

[SaurabhJha]

In D100051#2674674, @erichkeane wrote:

Hrmph... Phab ate my other comment, which was that the name EmitCastBetweenScalarTypes feels clunky. Does EmitScalarCast or EmitScalarScalarCast sound better and capture the meaning correctly?

Yeah, good idea. I'll rename it to EmitScalarCast.

[SaurabhJha]

Rename the function to EmitScalarCast and directly returning from if branches now

[SaurabhJha]

I don't have commit rights so if it looks good to you, please commit on my behalf.

[erichkeane]

Alright, validating it now, then I'll push.

[fhahn]

In D100051#2674849, @erichkeane wrote:

Alright, validating it now, then I'll push.

Thanks Erich!

[SaurabhJha]

In D100051#2674849, @erichkeane wrote:

Alright, validating it now, then I'll push.

Thanks very much.

clang/lib/CodeGen/CGExprScalar.cpp

struct ScalarConversionOpts {
 
     ScalarConversionOpts(clang::SanitizerSet SanOpts)
         : TreatBooleanAsSigned(false),
           EmitImplicitIntegerTruncationChecks(
               SanOpts.hasOneOf(SanitizerKind::ImplicitIntegerTruncation)),
           EmitImplicitIntegerSignChangeChecks(
               SanOpts.has(SanitizerKind::ImplicitIntegerSignChange)) {}
   };
+  Value *EmitCastBetweenScalarTypes(Value *Src, QualType SrcType,
+                                    QualType DstType, llvm::Type *SrcTy,
+                                    llvm::Type *DstTy,
+                                    ScalarConversionOpts Opts);
   Value *
   EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy,
                        SourceLocation Loc,
                        ScalarConversionOpts Opts = ScalarConversionOpts());
 
   /// Convert between either a fixed point and other fixed point or fixed point
   /// and an integer.
   Value *EmitFixedPointConversion(Value *Src, QualType SrcTy, QualType DstTy,
llvm::Constant *StaticArgs[] = {
       CGF.EmitCheckSourceLocation(Loc), CGF.EmitCheckTypeDescriptor(SrcType),
       CGF.EmitCheckTypeDescriptor(DstType),
       llvm::ConstantInt::get(Builder.getInt8Ty(), CheckKind)};
   // EmitCheck() will 'and' all the checks together.
   CGF.EmitCheck(Checks, SanitizerHandler::ImplicitConversion, StaticArgs,
                 {Src, Dst});
 }
 
+Value *ScalarExprEmitter::EmitCastBetweenScalarTypes(
+    Value *Src, QualType SrcType, QualType DstType, llvm::Type *SrcTy,
+    llvm::Type *DstTy, ScalarConversionOpts Opts) {
+  Value *Res = nullptr;
+
+  if (isa<llvm::IntegerType>(SrcTy)) {
+    bool InputSigned = SrcType->isSignedIntegerOrEnumerationType();
+    if (SrcType->isBooleanType() && Opts.TreatBooleanAsSigned) {
+      InputSigned = true;
+    }
+    if (isa<llvm::IntegerType>(DstTy))
+      Res = Builder.CreateIntCast(Src, DstTy, InputSigned, "conv");
+    else if (InputSigned)
+      Res = Builder.CreateSIToFP(Src, DstTy, "conv");
+    else
+      Res = Builder.CreateUIToFP(Src, DstTy, "conv");
+  } else if (isa<llvm::IntegerType>(DstTy)) {
+    assert(SrcTy->isFloatingPointTy() && "Unknown real conversion");
+    if (DstType->isSignedIntegerOrEnumerationType())
+      Res = Builder.CreateFPToSI(Src, DstTy, "conv");
+    else
+      Res = Builder.CreateFPToUI(Src, DstTy, "conv");
+  } else {
+    if (DstTy->getTypeID() < SrcTy->getTypeID())
+      Res = Builder.CreateFPTrunc(Src, DstTy, "conv");
+    else
+      Res = Builder.CreateFPExt(Src, DstTy, "conv");
+  }
+
+  return Res;
+}
+
 /// Emit a conversion from the specified type to the specified destination type,
 /// both of which are LLVM scalar types.
 Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
                                                QualType DstType,
                                                SourceLocation Loc,
                                                ScalarConversionOpts Opts) {
   // All conversions involving fixed point types should be handled by the
   // EmitFixedPoint family functions. This is done to prevent bloating up this
if (SrcTy->isFloatingPointTy()) {
         return Builder.CreateCall(
             CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_to_fp16, SrcTy), Src);
       // If the half type is supported, just use an fptrunc.
       return Builder.CreateFPTrunc(Src, DstTy);
     }
     DstTy = CGF.FloatTy;
   }
 
-  if (isa<llvm::IntegerType>(SrcTy)) {
-    bool InputSigned = SrcType->isSignedIntegerOrEnumerationType();
-    if (SrcType->isBooleanType() && Opts.TreatBooleanAsSigned) {
-      InputSigned = true;
-    }
-    if (isa<llvm::IntegerType>(DstTy))
-      Res = Builder.CreateIntCast(Src, DstTy, InputSigned, "conv");
-    else if (InputSigned)
-      Res = Builder.CreateSIToFP(Src, DstTy, "conv");
-    else
-      Res = Builder.CreateUIToFP(Src, DstTy, "conv");
-  } else if (isa<llvm::IntegerType>(DstTy)) {
-    assert(SrcTy->isFloatingPointTy() && "Unknown real conversion");
-    if (DstType->isSignedIntegerOrEnumerationType())
-      Res = Builder.CreateFPToSI(Src, DstTy, "conv");
-    else
-      Res = Builder.CreateFPToUI(Src, DstTy, "conv");
-  } else {
-    assert(SrcTy->isFloatingPointTy() && DstTy->isFloatingPointTy() &&
-           "Unknown real conversion");
-    if (DstTy->getTypeID() < SrcTy->getTypeID())
-      Res = Builder.CreateFPTrunc(Src, DstTy, "conv");
-    else
-      Res = Builder.CreateFPExt(Src, DstTy, "conv");
-  }
+  Res = EmitCastBetweenScalarTypes(Src, SrcType, DstType, SrcTy, DstTy, Opts);
 
   if (DstTy != ResTy) {
     if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
       assert(ResTy->isIntegerTy(16) && "Only half FP requires extra conversion");
       Res = Builder.CreateCall(
         CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_to_fp16, CGF.CGM.FloatTy),
         Res);
     } else {