[PATCH] D83833: [GISel] Add new GISel combiners for G_SELECT

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/Target/GlobalISel/Combine.td:206
 
+// Fold (undef ? x : y) -> x
+def select_undef_cmp: GICombineRule<
----------------
This matches InstSimplify, although I do wonder why the choice isn't to select the RHS in this case. Since we generally prefer to canonicalize lower complexity expressions to the RHS, it seems like that would generally be the cheaper choice. I do think this should match the InstSimplify behavior though


================
Comment at: llvm/include/llvm/Target/GlobalISel/Combine.td:227
+// Fold (false ? x : y) -> y
+def select_constant_cmp_matchdata : GIDefMatchData<"unsigned">;
+def select_constant_cmp: GICombineRule<
----------------
I've been thinking GIMatchData has the annoying property where every combine that uses the same thing needs to define its own GIDefMatchData.


================
Comment at: llvm/include/llvm/Target/GlobalISel/Combine.td:214
+
+// Fold (cmp ? x : undef) -> x
+// Fold (cmp ? undef : y) -> y
----------------
arsenm wrote:
> This may not be correct? See 1cf6f210a2ed87dcda2183fffd6f9aa17b5c493c
Probably should split this into its own patch since it's questionable?


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

https://reviews.llvm.org/D83833