[PATCH] D119654: [SDAG] enable binop identity constant folds for add/sub

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2148
+      }
+    }
     return false;
----------------
xbolva00 wrote:
> You can use Constantexpr::getBinOpIdentity to check if identity constant.
> 
> 
Sorry, this is SDNode and it seems there is no getBinOpIdentity() API. 
BTW, there is some regression on this patch. 
1. When the select can be combined with its operands, we don't need to invert the select folding. See below example.

```
; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx512vbmi2,+avx512vl --show-mc-encoding | FileCheck %s --check-prefixes=CHECK,X64
define <16 x i16> @test_int_x86_avx512_mask_vpshldv_w_256(<16 x i16> %x0, <16 x i16> %x1, <16 x i16>* %x2p, <16 x i16> %x4, i16 %x3) {
  %x2 = load <16 x i16>, <16 x i16>* %x2p
  %1 = call <16 x i16> @llvm.fshl.v16i16(<16 x i16> %x0, <16 x i16> %x1, <16 x i16> %x2)
  %2 = bitcast i16 %x3 to <16 x i1>
  %3 = select <16 x i1> %2, <16 x i16> %1, <16 x i16> %x0
  %4 = call <16 x i16> @llvm.fshl.v16i16(<16 x i16> %x0, <16 x i16> %x1, <16 x i16> %x4)
  %5 = bitcast i16 %x3 to <16 x i1>
  %6 = select <16 x i1> %5, <16 x i16> %4, <16 x i16> zeroinitializer
  %res3 = add <16 x i16> %3, %6
  ret <16 x i16> %res3
}

declare <16 x i16> @llvm.fshl.v16i16(<16 x i16> %x0, <16 x i16> %x1, <16 x i16> %x4)
```
2. The freeze node seems to prevent the sub combine for below case.

```
define <4 x i32> @test_srem_allones(<4 x i32> %X) nounwind {
  %srem = srem <4 x i32> %X, <i32 4294967295, i32 4294967295, i32 4294967295, i32 4294967295>
  %cmp = icmp eq <4 x i32> %srem, <i32 0, i32 0, i32 0, i32 0>
  %ret = zext <4 x i1> %cmp to <4 x i32>
  ret <4 x i32> %ret
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119654