[PATCH] D59881: Initial support for vectorization using MASSV (IBM MASS vector library)

Tue May 14 08:00:12 PDT 2019

jsji added a subscriber: chandlerc.
jsji added a comment.

Some more nit-picker, other than that, mostly good to me.

However, as Chandler mentioned in similar patch for AArch64 in https://reviews.llvm.org/D53927,

In D53927#1307196 <https://reviews.llvm.org/D53927#1307196>, @chandlerc wrote:

> Adding new support for a vector math library seems worth getting reviewed / approved by someone reasonably deeply familiar w/ our vectorization infra.

I would be good to have @hfinkel or @nemanjai or someone more familiar with vectorization infra to double confirm and approve it.

================
Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/massv-altivec.ll:2
+; RUN: opt -vector-library=MASSV -loop-vectorize -force-vector-interleave=1 -mattr=-altivec -S < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-n32:64" 
----------------
I would be great if we can add some comments to describe the overall test points, eg: this is for negative test of massv without altivec support. This apply to all testcases in this patch.

================
Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/massv-calls.ll:282
+}
+
+define void @exp_f64(double* nocapture %varray) {
----------------
jsji wrote:
> Missing some intrinsics test? @sqrt_f64_intrinsic?@sqrt_f32_intrinsic?
Still missing in latest patch?

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59881