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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2148
+      }
+    }
     return false;
----------------
LuoYuanke wrote:
> 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
> }
> ```
> You can use Constantexpr::getBinOpIdentity to check if identity constant.

We already have SelectionDAG::getNeutralElement - I wonder if adding a SelectionDAG::isNeutralElement helper sibling would be useful?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119654