kosarev added a comment.

Thanks for reviewing.



================
Comment at: lib/CodeGen/CGBuiltin.cpp:7865
   }
     // FIXME: Sharing loads & stores with 32-bit is complicated by the absence
     // of an Align parameter here.
----------------
SjoerdMeijer wrote:
> How about this FIXME? Is it still relevant? And does it need to be moved up? 
> Or perhaps better: move the code back here to minimise the diff?
Yes, it's still true for the vst builtins handled below. None of the vld/vst 
patches removes this comment, but it should go away with whatever is the one to 
be committed last.

Umm, it seems leaving the vld code here wouldn't make the diff smaller?


================
Comment at: test/CodeGen/arm-neon-vld.c:4
+// RUN:     FileCheck -check-prefixes=CHECK,CHECK-A64 %s
+// RUN: %clang_cc1 -triple armv8-none-linux-gnueabi -target-feature +neon \
+// RUN:     -S -disable-O0-optnone -emit-llvm -o - %s | opt -S -mem2reg | \
----------------
SjoerdMeijer wrote:
> Should this be armv7?
There are more ARMv8 vld intrinsics that we currently support only in A64 so I 
was going to add tests for them here. I'm not sure if we want to test 
availability of NEON intrinsics for various architectures with codegen tests 
like this one or have some separate tests in sema.


https://reviews.llvm.org/D47121



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to