LukeGeeson marked 2 inline comments as done.
LukeGeeson added inline comments.
================
Comment at: lib/Sema/SemaChecking.cpp:1409
- switch (BuiltinID) {
-#define GET_NEON_OVERLOAD_CHECK
-#include "clang/Basic/arm_neon.inc"
----------------
SjoerdMeijer wrote:
> Why do we need to remove this?
We don't actually need this one, inspecting include/clang/Basic/arm_fp16.inc
after building shows an empty set of #ifdefs pragmas - ie nothing is generated
and hence nothing happens.
Will leave in for continuity, however this may be redundant
================
Comment at: test/Sema/aarch64-neon-fp16-ranges.c:1
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon
-fallow-half-arguments-and-returns -target-feature +fullfp16 -ffreestanding
-fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon
-fallow-half-arguments-and-returns -target-feature +fullfp16 -ffreestanding
-fsyntax-only -verify %s
----------------
SjoerdMeijer wrote:
> Nit: target feature fullfp16 implies ARMv8 FP, so I think you can remove
> +neon; just a tiny optimisation to make the command line shorter (same below).
That doesn't seem to work, removing neon introduces hundreds of errors around
incompatible types (including things this diff doesn't change) which would
suggest maybe neon doesn't cover everything?
================
Comment at: test/Sema/aarch64-neon-fp16-ranges.c:39
+
+void test_vcvt_su_f(int64_t a){
+ vcvth_n_s16_f16(a, 1);
----------------
SjoerdMeijer wrote:
> why is this is 'a' an int64_t? Should this not be float16_t?
Agreed. Fixed.
https://reviews.llvm.org/D47592
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
