[PATCH] D67345: [InstCombine] Allow values with multiple users in SimplifyDemandedVectorElts

Mon Sep 30 09:59:50 PDT 2019

nhaehnle added a comment.

This is a useful change, but there is an unfortunate asymmetry here in how the code is structured: in addition to `extractelement`, we could also have `shufflevector` users (or masked stores etc.). Presumably we'd be able to handle all of those together without duplicating the code. Is there a way to take this into account?

Also, two comments inline.

================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:296
+
+          if (!EEIIndexC || !EEIIndexC->getValue().ule(NumElts)) {
+            AllUsesAreValidConstEEI = false;
----------------
`ult` instead of `ule`.

================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:304-305
+          APInt UndefElts(NumElts, 0);
+          if (Value *V = SimplifyDemandedVectorElts(SrcVec, DemandedElts,
+                                                    UndefElts, 0 /* Depth */, true /* AllowMultipleUsers */)) {
+           if (V != SrcVec)
----------------
Skip this if DemandedElts is allOnes.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67345