[PATCH] D113107: Support of expression granularity for _Float16.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.


================
Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:42
 _Float16 add2(_Float16 a, _Float16 b, _Float16 c) {
   return a + b + c;
 }
----------------
Missing the same ternary operation test in complex tests?


================
Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:207
+// CHECK-NEXT:    [[EXT:%.*]] = fpext half [[TMP0]] to float
+// CHECK-NEXT:    store float [[EXT]], ptr [[RETVAL]], align 2
+// CHECK-NEXT:    [[TMP1:%.*]] = load half, ptr [[RETVAL]], align 2
----------------
Not sure if we need a fptrunc and store the half value. The following tests have the same problem.


================
Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:369-370
+// CHECK-NEXT:    [[ADD_R:%.*]] = fadd float [[EXT]], [[FNEG]]
+// CHECK-NEXT:    store float [[ADD_R]], ptr [[RETVAL]], align 2
+// CHECK-NEXT:    [[TMP1:%.*]] = load half, ptr [[RETVAL]], align 2
+// CHECK-NEXT:    ret half [[TMP1]]
----------------
The operations don't look correct. The following tests have the same problem.


================
Comment at: clang/test/CodeGen/X86/Float16-complex.c:2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefixes=AVX
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefixes=X86
----------------
AVX/X86 are not good names. Maybe NATIVE/PROMOTE?


================
Comment at: clang/test/CodeGen/volatile-1.c:195
   ci=ci=ci;
-  // CHECK-NEXT: [[T:%.*]] = load volatile [[INT]], [[INT]]* getelementptr inbounds ([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
+  // CHECK: [[T:%.*]] = load volatile [[INT]], [[INT]]* getelementptr inbounds ([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
   // CHECK-NEXT: store volatile [[INT]] [[T]], [[INT]]* getelementptr inbounds ([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
----------------
What's happening here? Better to add a comment if it is intended.


================
Comment at: clang/test/CodeGenCXX/volatile-1.cpp:241
   __imag ci = __imag ci = __imag ci;
-  // CHECK-NEXT: [[T:%.*]] = load volatile [[INT]], [[INT]]* getelementptr inbounds ([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
+  // CHECK: [[T:%.*]] = load volatile [[INT]], [[INT]]* getelementptr inbounds ([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
   // CHECK-NEXT: store volatile [[INT]] [[T]], [[INT]]* getelementptr inbounds ([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
----------------
ditto.


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

https://reviews.llvm.org/D113107