[PATCH] D25956: Implement vector_insert_exp builtins - clang portion

Eric Christopher via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 25 13:20:42 PDT 2016


echristo accepted this revision.
echristo added a reviewer: echristo.
echristo added a comment.
This revision is now accepted and ready to land.

Agreeing with Nemanja on a comment, once they're happy this LGTM.

-eric



================
Comment at: lib/Headers/altivec.h:2500
 
+#ifdef __VSX__
+static __inline__ vector double  __ATTRS_o_ai
----------------
nemanjai wrote:
> nemanjai wrote:
> > Hmm... I think
> > 
> > ```
> > && __POWER9_VECTOR__
> > ```
> Actually, I take that back. It should only be __POWER9_VECTOR__.
> This is important to get right because using this on a P8 or older machine will cause a run-time rather than a compile-time failure (i.e. it will compile just fine and executing the code would result in an illegal instruction).
+1


================
Comment at: test/CodeGen/builtins-ppc-p9vector.c:704
+// CHECK-BE-NEXT: ret <2 x double>
+// CHECK: @llvm.ppc.vsx.xviexpdp(<2 x i64> %{{[0-9]+}}, <2 x i64>
+// CHECK-NEXT: ret <2 x double>
----------------
nemanjai wrote:
> I don't think this is guaranteed to succeed. If I remember correctly, Clang names temporaries differently with and without asserts. Maybe it wasn't asserts, but I remember something made Clang choose different names for temporaries (rather than %0, %1, etc.). So I think this should suffice as the regex pattern:
> `%{{.+}}`
Correct.


https://reviews.llvm.org/D25956





More information about the llvm-commits mailing list