[PATCH] D21703: Scalarizer: Support scalarizing intrinsics

[Matt Arsenault via llvm-commits]

arsenm added inline 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);
----------------
mehdi_amini wrote:
> 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.
I don't think it's that surprising. There's no obvious scalarizaton/vectorization for something like that. Not many places really care about the general types of intrinsics besides the vectorizers.

================
Comment at: lib/Transforms/Scalar/Scalarizer.cpp:439
@@ +438,3 @@
+      ScalarOperands[I] = CI.getOperand(I);
+    else {
+      Scattered[I] = scatter(&CI, CI.getOperand(I));
----------------
mehdi_amini wrote:
> 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".
It also checks the operand index. This is just the existing mechanism, I guess it would also work to check the type


https://reviews.llvm.org/D21703