[PATCH] D149918: [InstCombine] Add oneuse checks to shr + cmp constant folds.

Thu May 4 18:01:17 PDT 2023

aemerson created this revision.
aemerson added reviewers: spatel, dmgreen, nikic.
aemerson added a project: LLVM.
Herald added subscribers: StephenFan, hiraditya, kristof.beyls.
Herald added a project: All.
aemerson requested review of this revision.

This change has virtually no code size regressions on the llvm test suite (+ SPECs)
while having these improvements (measured with -Os on Darwin arm64):

  External/S.../CFP2006/450.soplex/450.soplex    214024.00      213920.00     -0.0%
  External/S...7speed/641.leela_s/641.leela_s     93412.00       93348.00     -0.1%
  External/S...17rate/541.leela_r/541.leela_r     93412.00       93348.00     -0.1%
  MultiSourc.../Applications/JM/lencod/lencod    426044.00      425748.00     -0.1%
  MultiSourc...rks/mediabench/gsm/toast/toast     20436.00       20416.00     -0.1%
  MultiSourc...ench/telecomm-gsm/telecomm-gsm     20436.00       20416.00     -0.1%
  MultiSourc...Prolangs-C/assembler/assembler     16172.00       16156.00     -0.1%
  MultiSourc...nch/mpeg2/mpeg2dec/mpeg2decode     35332.00       35256.00     -0.2%
  SingleSour...Adobe-C++/stepanov_abstraction      6904.00        6888.00     -0.2%
  External/SPEC/CINT2000/254.gap/254.gap         366060.00      365132.00     -0.3%
  MultiSourc...-ProxyApps-C++/PENNANT/PENNANT     79688.00       79484.00     -0.3%
  External/S...NT2006/464.h264ref/464.h264ref    352044.00      351132.00     -0.3%
  SingleSour...arks/Adobe-C++/functionobjects     15524.00       15480.00     -0.3%
  SingleSour...arks/Adobe-C++/stepanov_vector     10728.00       10696.00     -0.3%
  SingleSour...ks/Misc-C++/stepanov_container     16900.00       16848.00     -0.3%
  MultiSource/Applications/oggenc/oggenc         124184.00      123780.00     -0.3%
  SingleSour...tout-C++/Shootout-C++-wordfreq      7060.00        7036.00     -0.3%
  MultiSourc...ity-rijndael/security-rijndael      8976.00        8936.00     -0.4%
  MultiSource/Benchmarks/McCat/18-imp/imp          9816.00        9772.00     -0.4%
  SingleSour...chmarks/Misc-C++/stepanov_v1p2      1772.00        1764.00     -0.5%
  MultiSourc...iabench/g721/g721encode/encode      5492.00        5464.00     -0.5%
  MultiSourc...rks/McCat/03-testtrie/testtrie      1364.00        1344.00     -1.5%
  SingleSour.../execute/GCC-C-execute-pr42833       400.00         364.00     -9.0%

Doing so also prevents a regression described in https://reviews.llvm.org/D143624


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149918

Files:
  llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
  llvm/test/Transforms/InstCombine/icmp-shr-lt-gt.ll
  llvm/test/Transforms/InstCombine/icmp-shr.ll


Index: llvm/test/Transforms/InstCombine/icmp-shr.ll
===================================================================

--- llvm/test/Transforms/InstCombine/icmp-shr.ll
+++ llvm/test/Transforms/InstCombine/icmp-shr.ll
@@ -576,6 +576,19 @@
   ret i1 %r
 }
 
+define i1 @ashr_ugt_0_multiuse(i4 %x, ptr %p) {
+; CHECK-LABEL: @ashr_ugt_0_multiuse(
+; CHECK-NEXT:    [[S:%.*]] = ashr i4 [[X:%.*]], 1
+; CHECK-NEXT:    [[R:%.*]] = icmp ugt i4 [[X]], 1
+; CHECK-NEXT:    store i4 [[S]], ptr [[P:%.*]], align 1
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %s = ashr i4 %x, 1
+  %r = icmp ugt i4 %s, 0 ; 0b0000
+  store i4 %s, ptr %p
+  ret i1 %r
+}
+
 define i1 @ashr_ugt_1(i4 %x) {
 ; CHECK-LABEL: @ashr_ugt_1(
 ; CHECK-NEXT:    [[R:%.*]] = icmp ugt i4 [[X:%.*]], 3
@@ -764,6 +777,19 @@
   ret i1 %r
 }
 
+define i1 @ashr_ult_2_multiuse(i4 %x, ptr %p) {
+; CHECK-LABEL: @ashr_ult_2_multiuse(
+; CHECK-NEXT:    [[S:%.*]] = ashr i4 [[X:%.*]], 1
+; CHECK-NEXT:    [[R:%.*]] = icmp ult i4 [[X]], 4
+; CHECK-NEXT:    store i4 [[S]], ptr [[P:%.*]], align 1
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %s = ashr i4 %x, 1
+  %r = icmp ult i4 %s, 2 ; 0b0010
+  store i4 %s, ptr %p
+  ret i1 %r
+}
+
 ; negative test
 
 define i1 @ashr_ult_3(i4 %x) {
Index: llvm/test/Transforms/InstCombine/icmp-shr-lt-gt.ll
===================================================================
--- llvm/test/Transforms/InstCombine/icmp-shr-lt-gt.ll
+++ llvm/test/Transforms/InstCombine/icmp-shr-lt-gt.ll
@@ -897,6 +897,19 @@
   ret i1 %c
 }
 
+define i1 @ashrsgt_01_00_multiuse(i4 %x, ptr %p) {
+; CHECK-LABEL: @ashrsgt_01_00_multiuse(
+; CHECK-NEXT:    [[S:%.*]] = ashr i4 [[X:%.*]], 1
+; CHECK-NEXT:    [[C:%.*]] = icmp sgt i4 [[S]], 0
+; CHECK-NEXT:    store i4 [[S]], ptr [[P:%.*]], align 1
+; CHECK-NEXT:    ret i1 [[C]]
+;
+  %s = ashr i4 %x, 1
+  %c = icmp sgt i4 %s, 0
+  store i4 %s, ptr %p
+  ret i1 %c
+}
+
 define i1 @ashrsgt_01_01(i4 %x) {
 ; CHECK-LABEL: @ashrsgt_01_01(
 ; CHECK-NEXT:    [[C:%.*]] = icmp sgt i4 [[X:%.*]], 3
Index: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -2375,7 +2375,7 @@
   // constant-value-based preconditions in the folds below, then we could assert
   // those conditions rather than checking them. This is difficult because of
   // undef/poison (PR34838).
-  if (IsAShr) {
+  if (IsAShr && Shr->hasOneUse()) {
     if (IsExact || Pred == CmpInst::ICMP_SLT || Pred == CmpInst::ICMP_ULT) {
       // When ShAmtC can be shifted losslessly:
       // icmp PRED (ashr exact X, ShAmtC), C --> icmp PRED X, (C << ShAmtC)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149918.519707.patch
Type: text/x-patch
Size: 2713 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230505/6174a938/attachment.bin>
