[PATCH] D56900: [Fixed Point Arithmetic] Fixed Point and Integer Conversions

Bjorn Pettersson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 1 00:19:29 PST 2019

bjope added inline comments.

Comment at: clang/test/Frontend/fixed_point_conversions.c:45
+short _Accum sa_const9 = 2u; // DEFAULT-DAG: @sa_const9 = {{.*}}global i16 256, align 2
+unsigned short _Accum usa_const3 = 2;
Perhaps we should verify that we do not mess up signed values when using the -fpadding-on-unsigned-fixed-point option (having CHECK, DEFAULT and SAME as check prefixes)?

(nit: I also see that we now use different names for this check prefixes in different test files, which could be confusing. Why is it called SAME here?) 

Comment at: clang/test/Frontend/fixed_point_conversions.c:66
+_Sat short _Accum sat_sa_const3 = 256;  // DEFAULT-DAG: @sat_sa_const3 = {{.*}}global i16 32767, align 2
+_Sat short _Accum sat_sa_const4 = -257; // DEFAULT-DAG: @sat_sa_const4 = {{.*}}global i16 -32768, align 2
A little bit confusing to be with this mix of having checks left-aligned (on separate lines) and some checks appended like this.
I can see that it already is like that before, so maybe it should be cleaned up in a separate commit (before this commit).

(I'm not so familiar with clang tests, so maybe it is common to have the FileCheck checks at the end of the line. But I've never seen that in llvm.)

  rC Clang



More information about the cfe-commits mailing list