[PATCH] D72958: [InstSimplify] fold select of vector constants that include undef elements

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4044
+        NewC.push_back(TEltC);
+      }
+    }
----------------
nikic wrote:
> I'm having a bit of a hard time convincing myself that this is always correct. In particular, the relationship between isElementWiseEqual(), which is based on `ConstantExpr::getICmp()` and this code, which is based on simple identity comparison, is not entirely clear. If the vector includes complex constant expressions, and `ConstantExpr::getICmp()` manages to fold that code while the identity comparison fails, this would trigger an assertion failure here. Or is there an invariant that this cannot happen?
I've tried to concoct an example of this:

```
@g = global i32 0

define <3 x i32> @sel_constant_expression(<3 x i1> %cond) {
  %r = select <3 x i1> %cond, <3 x i32> <i32 ptrtoint (i32* @g to i32), i32 0, i32 1>, <3 x i32> <i32 bitcast (<2 x i16> bitcast (i32 ptrtoint (i32* @g to i32) to <2 x i16>) to i32), i32 0, i32 undef>
  ret <3 x i32> %r
}
```

...but the 2nd ConstantExpr get constant folded into the reduced form of the 1st (the bitcasts go away). So we can either leave this as-is, or we could bail out on ConstantExpr to be safe(r)? I can't imagine that this will ever make a difference in any real-world code.


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

https://reviews.llvm.org/D72958