[PATCH] D72722: [FPEnv] [SystemZ] Platform-specific builtin constrained FP enablement

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand added a comment.

A few comments (see inline) -- otherwise this looks good to me, thanks!



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13354
+      return Builder.CreateFNeg(Builder.CreateConstrainedFPCall(F, {X, Y,  Z}), "neg");
+
+    } else {
----------------
Spurious empty line.


================
Comment at: clang/test/CodeGen/builtins-systemz-vector2-constrained.c:25
+  // CHECK: [[NEG:%[^ ]+]] = fneg <2 x double> {{.*}}
+  // CHECK:  [[RES:%[^ ]+]] = call <2 x double> @llvm.experimental.constrained.fma.v2f64(<2 x double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> [[NEG]], metadata !{{.*}}, metadata !{{.*}})
+  // CHECK: fneg <2 x double> [[RES]]
----------------
Why is it that this one has metadata nodes and all the others do not?   Do they really not have metadata (why?) or are you just not checking for them?


================
Comment at: clang/test/CodeGen/builtins-systemz-zvector2-constrained.c:12
+// FIXME: The lack of constant folding for strict fp breaks this test.
+// See label FIXME-CHECK-ASM.
+
----------------
This was caused by unnecessary implicit conversions in the vecintrin.h code, I've now checked in cebba7c to remove those.   Can you rebase?  This FIXME should no longer be necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72722