[PATCH] D43105: [RISCV] Enable __int128_t and __uint128_t through clang flag

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.

I've added two suggestions on further tweaking the tests which I think it would be worth adopting. Looks good to me.



================
Comment at: test/CodeGen/riscv32-abi.c:424-430
+
+// RUN: %clang_cc1 -triple riscv32 -fforce-enable-int128 \
+// RUN: -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-FORCEINT128-LABEL
+#ifdef __SIZEOF_INT128__
+// CHECK-FORCEINT128-LABEL: define i128 @f_scalar_5(i128 %x)
+__int128_t f_scalar_5(__int128_t x) { return x; }
+#endif
----------------
I'd probably insert this around line 26 (just after f_scalar_4) and put the RUN line at the very top of the file with -check-prefixes=CHECK,CHECK-FORCEINT128. This would demonstrate and verify that all other ABI lowering remains totally unaffected, which probably can't hurt. Note that `$CHECKPREFIX-LABEL` is a [FileCheck feature](https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-label-directive), so the prefix should be defined without `-LABEL`.


================
Comment at: test/Preprocessor/init.c:10213-10216
+// RUN: %clang_cc1 -E -dM -ffreestanding \
+// RUN: -triple=riscv32 -fforce-enable-int128 < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefix=RISCV32-INT128 %s
+// RISCV32-INT128: #define __SIZEOF_INT128__ 16
----------------
It would be slightly better to define this in a similar way to the RISCV32-LINUX checks, so that the test also picks up the fact that other defines (such as _INTMAX_WIDTH__) remain unmodified.


Repository:
  rC Clang

https://reviews.llvm.org/D43105