[PATCH] D137934: [InstCombine] Fold extractelt with select of constants

[Thomas Symalla via Phabricator via llvm-commits]

tsymalla added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:401-402
 
+  // extractelt (select %x, <vec1>, <vec2>), %const ->
+  // select %x, <vec1>[%const], <vec2>[%const]
+  // FIXME: Support multiple select operands during constant folding:
----------------
foad wrote:
> Why do you write `<vec1>` instead of `%vec1`? I found it confusing to have two different syntaxes for variables in the same line.
I wanted to use the vector-notation `<v_1, ..., v_n`, but I can adjust it.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:403-406
+  // FIXME: Support multiple select operands during constant folding:
+  // extractelt (select %c, %<vec1>, %<vec2>), (select %c, %c1, %c2)
+  // This is currently not possible because constant folding will reach
+  // an unreachable assertion if it doesn't find a constant operand.
----------------
foad wrote:
> I don't really understand this comment. "FIXME" suggests that something is broken. What is broken? What would you like to fold `extractelt (select %c, %<vec1>, %<vec2>), (select %c, %c1, %c2)` into?
Currently, cases like

`extractelt (select %c, %<vec1>, %<vec2>), (select %c, 3, 4)` are not handled. If such instruction sequence leads to OOB behavior at compile time, this could be caught here.
I'm going to adjust the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137934