[PATCH] D82961: [Scalarizer] Variable insert handling (PR46524)

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I like the flag idea, but I would expect this to be the default. If I read scalarizer and I enable it, I would expect the code to be scalarized, not partially scalarized based on a heuristic that may or may not be to my liking. The option to opt-out of some scalarization is sensible though.

Out of curiosity, did u try we fold this properly if the index is substituted afterwards with a constant?

> I figure the resulting code is less poisonous (insertelement can generate poison, but the select sequence won't). I guess that is ok.

Agreed.



================
Comment at: llvm/lib/Transforms/Scalar/Scalarizer.cpp:759
+  if (isa<Constant>(InsIdx))
+    return false;
+
----------------
I do not understand why we skip constant indices. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82961