[PATCH] D90113: [DAGCombiner] Fold BinOp into Select containing identity constant

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

There's a large number of regressions in here that still need addressing - have you had any luck nailing any of them down?



================
Comment at: llvm/test/CodeGen/AMDGPU/amdgpu-codegenprepare-idiv.ll:339-341
+; GCN-NEXT:    v_add_i32_e32 v2, vcc, 1, v3
 ; GCN-NEXT:    v_cmp_ge_f32_e64 vcc, |v1|, v0
+; GCN-NEXT:    v_cndmask_b32_e32 v0, v3, v2, vcc
----------------
foad wrote:
> Regression here and in lots of other tests below. It looks like it's no longer using an add-with-carry instruction to implement x+(cond?1:0).
Is this just a generic combine we're missing or would this need to be fixed with an isel pattern?


================
Comment at: llvm/test/CodeGen/X86/avx-select.ll:26
   %res = xor <8 x i32> %b, %selres
   ret <8 x i32> %res
 }
----------------
I'm not sure if these tests are testing what there were supposed to anymore?


================
Comment at: llvm/test/CodeGen/X86/binop-identity.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=x86_64-pc-linux -x86-cmov-converter=true -verify-machineinstrs < %s | FileCheck %s
+
----------------
Maybe pre-commit these tests to trunk and rebase this patch to show the delta?

I'm not sure you need -verify-machineinstrs

We're going to need a binop-identity-vector.ll set of tests as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90113