[RISCV] Fix int16 -> __fp16 conversion code gen

[kito-cheng]

Summary

clang emit wrong code sequence for int16(short) to __fp16 conversion,
and that should fix the code gen directly is the right way I think,
but I found there is a FIXME comment in clang/Basic/TargetInfo.h say
that's should be removed in future so I think just let swich to using
generic LLVM IR rather than llvm.convert.to.fp16 intrinsics code gen
path is enough.

/// Check whether llvm intrinsics such as llvm.convert.to.fp16 should be used
/// to convert to and from __fp16.
/// FIXME: This function should be removed once all targets stop using the
/// conversion intrinsics.
virtual bool useFP16ConversionIntrinsics() const {
  return true;
}

[khchen]

It's seem like all targets need to return false in useFP16ConversionIntrinsics for correctness of int16 -> __fp16 conversion?

[craig.topper]

I'm not opposed to this patch, but here's some additional info

I think this is failing because we hit this code in CGExprScalar.cpp. Where SrcTy and DstTy are both i16 when using the conversion intrinsics.

// Ignore conversions like int -> uint.
if (SrcTy == DstTy) {
  llvm::dbgs() << "ctopper20\n";
  if (Opts.EmitImplicitIntegerSignChangeChecks)
    EmitIntegerSignChangeCheck(Src, NoncanonicalSrcType, Src,
                               NoncanonicalDstType, Loc);

  return Src;
}

Something like this seems to fix it.

diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index d3fe04d5a791..62662bf860c2 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1339,7 +1339,7 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
   }
 
   // Ignore conversions like int -> uint.
-  if (SrcTy == DstTy) {
+  if (SrcTy == DstTy && !DstType->isHalfType()) {
     if (Opts.EmitImplicitIntegerSignChangeChecks)
       EmitIntegerSignChangeCheck(Src, NoncanonicalSrcType, Src,
                                  NoncanonicalDstType, Loc);

[craig.topper]

LGTM

clang/lib/Basic/Targets/RISCV.h

 //===--- RISCV.h - Declare RISCV target feature support ---------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
initFeatureMap(llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags,
                  const std::vector<std::string> &FeaturesVec) const override;
 
   bool hasFeature(StringRef Feature) const override;
 
   bool handleTargetFeatures(std::vector<std::string> &Features,
                             DiagnosticsEngine &Diags) override;
 
   bool hasBitIntType() const override { return true; }
+
+  bool useFP16ConversionIntrinsics() const override {
+    return false;
+  }
 };
 class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
 public:
   RISCV32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : RISCVTargetInfo(Triple, Opts) {
     IntPtrType = SignedInt;
     PtrDiffType = SignedInt;
     SizeType = UnsignedInt;

clang/test/CodeGen/RISCV/__fp16-convert.c

 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -triple riscv64 -emit-llvm %s -o - \
 // RUN:   | FileCheck %s
 
 __fp16 y;
 short z;
 // CHECK-LABEL: @bar1(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:    [[TMP0:%.*]] = load i16, ptr @y, align 2
-// CHECK-NEXT:    [[TMP1:%.*]] = call float @llvm.convert.from.fp16.f32(i16 [[TMP0]])
-// CHECK-NEXT:    [[CONV:%.*]] = fptosi float [[TMP1]] to i16
-// CHECK-NEXT:    store i16 [[CONV]], ptr @z, align 2
+// CHECK-NEXT:    [[TMP0:%.*]] = load half, ptr @y, align 2
+// CHECK-NEXT:    [[CONV:%.*]] = fpext half [[TMP0]] to float
+// CHECK-NEXT:    [[CONV1:%.*]] = fptosi float [[CONV]] to i16
+// CHECK-NEXT:    store i16 [[CONV1]], ptr @z, align 2
 // CHECK-NEXT:    ret void
 //
 void bar1(){
     z = y;
 }
 // CHECK-LABEL: @bar2(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = load i16, ptr @z, align 2
-// CHECK-NEXT:    store i16 [[TMP0]], ptr @y, align 2
+// CHECK-NEXT:    [[CONV:%.*]] = sitofp i16 [[TMP0]] to float
+// CHECK-NEXT:    [[CONV1:%.*]] = fptrunc float [[CONV]] to half
+// CHECK-NEXT:    store half [[CONV1]], ptr @y, align 2
 // CHECK-NEXT:    ret void
 //
 void bar2(){
     y = z;
 }