[PATCH] D109817: [AggressiveInstCombine] Add `phi` nodes support to `TruncInstCombine`

Fri Dec 10 01:02:01 PST 2021

anton-afanasyev added a comment.

In D109817#3060903 <https://reviews.llvm.org/D109817#3060903>, @RKSimon wrote:

> In D109817#3043432 <https://reviews.llvm.org/D109817#3043432>, @anton-afanasyev wrote:
>
>> Ping.
>> Though this patch makes AIC more complicated (using general graph instead of expression DAG), it spreads AIC outside one BB. It impacts ~30 tests of test suite.
>
> Are there any notable diffs (codegen/perf) in those tests?

I don't think that such diffs could be called "notable", what I see is rather controversal impact in code size:

  $ ~/llvm/test-suite/utils/compare.py -a --filter-short -m size..text result_base.json vs result_exp.json
  Tests: 2889
  Short Running: 465 (filtered out)
  Remaining: 2424
  Metric: size..text

  Program                                        lhs        rhs        diff 
   test-suite.../Benchmarks/Ptrdist/bc/bc.test   46658.00   46722.00    0.1%
   test-suite...terchange/LoopInterchange.test   232802.00  232818.00   0.0%
   test-suite...eProcessing/Dilate/Dilate.test   236098.00  236114.00   0.0%
   test-suite...Filtering/BilateralFilter.test   236498.00  236514.00   0.0%
   test-suite...sion/AnisotropicDiffusion.test   237202.00  237218.00   0.0%
   test-suite.../Builtins/Int128/Builtins.test   237218.00  237234.00   0.0%
   test-suite...terpolation/Interpolation.test   237458.00  237474.00   0.0%
   test-suite...ImageProcessing/Blur/blur.test   239058.00  239074.00   0.0%
   test-suite...eProcessing/Dither/Dither.test   239842.00  239858.00   0.0%
   test-suite...oBenchmarks/harris/harris.test   241122.00  241138.00   0.0%
   test-suite...LPVectorizationBenchmarks.test   256626.00  256642.00   0.0%
   test-suite...opVectorizationBenchmarks.test   353330.00  353346.00   0.0%
   test-suite...MemFunctions/MemFunctions.test   371794.00  371810.00   0.0%
   test-suite...SubsetBRawLoops/lcalsBRaw.test   421026.00  421042.00   0.0%
   test-suite...BLambdaLoops/lcalsBLambda.test   421282.00  421298.00   0.0%
   test-suite...SubsetARawLoops/lcalsARaw.test   430978.00  430994.00   0.0%
   test-suite...ALambdaLoops/lcalsALambda.test   432050.00  432066.00   0.0%
   test-suite...CLambdaLoops/lcalsCLambda.test   434514.00  434530.00   0.0%
   test-suite...SubsetCRawLoops/lcalsCRaw.test   434690.00  434706.00   0.0%
   test-suite...ications/JM/lencod/lencod.test   756738.00  756754.00   0.0%
   test-suite...itcode/Regression/fft/fft.test   7425490.00 7425522.00  0.0%
   test-suite...ute/GCC-C-execute-loop-13.test   546.00     546.00      0.0%
  ...
   test-suite.../GCC-C-execute-20000403-1.test   450.00     450.00      0.0%
   test-suite...lications/ClamAV/clamscan.test   531321.00  531289.00  -0.0%
   test-suite...nsumer-lame/consumer-lame.test   175042.00  175026.00  -0.0%
   test-suite...nsumer-jpeg/consumer-jpeg.test   167186.00  167170.00  -0.0%
   test-suite...pplications/oggenc/oggenc.test   252210.00  252162.00  -0.0%
   test-suite...lications/sqlite3/sqlite3.test   496770.00  496530.00  -0.0%
   test-suite...ests/Vectorizer/gcc-loops.test   21330.00   21314.00   -0.1%
   test-suite...s/MallocBench/cfrac/cfrac.test   20754.00   20722.00   -0.2%
   test-suite...itBench/uudecode/uudecode.test   1538.00    1522.00    -1.0%
   Geomean difference                                                  -0.0%

Therefore I would abandon this change until we see a strict need of it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109817