bryanpkc added inline comments.

================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:726
 
-    if (Feature == "+sme") {
-      HasSME = true;
----------------
sdesmalen wrote:
> Why did you remove this?
The `Feature == "+sme"` case is also handled below on line 782. Perhaps we 
should delete this block and add `HasFullFP16` below.


================
Comment at: clang/test/CodeGen/aarch64-sme-intrinsics/acle_sme_ld1.c:16
+//
+__attribute__((arm_streaming)) void test_svld1_hor_za8(uint32_t slice_base, 
svbool_t pg, const void *ptr) {
+  svld1_hor_za8(0, slice_base, 0, pg, ptr);
----------------
sdesmalen wrote:
> Hi @bryanpkc, would you be happy to remove the dependence on the attributes 
> patch for now, so that we can move forward to review/land this patch series?
> 
> I'm thinking of doing something like:
> 
>   // RUN: %clang_cc1 -DDISABLE_SME_ATTRIBUTES ....
>   ...
>   
>   #ifdef DISABLE_SME_ATTRIBUTES
>   #define ARM_STREAMING_ATTR
>   #else
>   #define ARM_STREAMING_ATTR __attribute__((arm_streaming))
>   #endif
>   
>   ...
>   
>   ARM_STREAMING_ATTR void test_svld1_hor_za16(uint32_t slice_base, svbool_t 
> pg, const void *ptr) {
>     svld1_hor_za16(0, slice_base, 0, pg, ptr);
>     svld1_hor_za16(1, slice_base, 7, pg, ptr);
>   }
> 
> With that the tests all pass, and when the attribute patches have landed we 
> can just remove the `-DDISABLE_SME_ATTRIBUTES` from the RUN lines.
Sounds good, I will give that a shot. If we can unblock this patch, I will also 
rebase and update the rest of the patch series. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127910

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

Reply via email to