[PATCH] D21703: Scalarizer: Support scalarizing intrinsics

[Mehdi AMINI via llvm-commits]

mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.

LGTM, with two comments


================
Comment at: lib/Transforms/Scalar/Scalarizer.cpp:437
@@ +436,3 @@
+  for (unsigned I = 0; I != NumArgs; ++I) {
+    if (hasVectorInstrinsicScalarOpd(ID, I))
+      ScalarOperands[I] = CI.getOperand(I);
----------------
The comment seems truncated (and too long for a single line), clang-format?

I'm surprised no intrinsic are breaking this, is it an assumption baked everywhere in the optimizer? 
It makes the code fragile to future intrinsics.

================
Comment at: lib/Transforms/Scalar/Scalarizer.cpp:439
@@ +438,3 @@
+      ScalarOperands[I] = CI.getOperand(I);
+    else {
+      Scattered[I] = scatter(&CI, CI.getOperand(I));
----------------
Why using this `hasVectorInstrinsicScalarOpd()`? 
I'm not even sure to understand its implementation by the way, it says `Identifies if the intrinsic has a scalar operand` but only check the intrinsic id for a specific list. That sounds quite fragile? The type seems much more likely to be "the source of truth".


https://reviews.llvm.org/D21703