[TargetPassConfig] Add CanonicalizeFreezeInLoops before LSR

[aqjune]

Summary

This patch adds CanonicalizeFreezeInLoops before LSR.
Relevant patch: https://reviews.llvm.org/D77523

[lebedev.ri]

This deserves a PhaseOrdering test (going from the most underoptimized but minimized IR, -O3)

[aqjune]

Add a test with llc -O3 showing that freeze does not affect codegen

[aqjune]

Update the test so it diverges without CanonicalizeFreezeInLoops

[nikic]

llvm/test/Transforms/CanonicalizeFreezeInLoops/aarch64.ll

4 ↗

(On Diff #255370)

Probably needs REQUIRES?

[aqjune]

llvm/test/Transforms/CanonicalizeFreezeInLoops/aarch64.ll

4 ↗

(On Diff #255370)

I'm not familiar with REQUIRES: directive, would this work?

; REQUIRES: arm-registered-target

[xbolva00]

llvm/test/Transforms/CanonicalizeFreezeInLoops/aarch64.ll

1 ↗

(On Diff #255370)

Precommit testcase?

[nikic]

llvm/test/Transforms/CanonicalizeFreezeInLoops/aarch64.ll

4 ↗

(On Diff #255370)

I believe for this triple it should be aarch64-registered-target.

[aqjune]

Add REQUIRES

[aqjune]

llvm/test/Transforms/CanonicalizeFreezeInLoops/aarch64.ll

1 ↗

(On Diff #255370)

Once D77523 is accepted, I'll precommit this test.

[aqjune]

Precommit test

[aqjune]

As the main patch is landed, probably it is okay to land this too?
If there is a non-trivial regression, I can temporarily revert this and work on it.

[aqjune]

Rebase

[aqjune]

ping

I evaluated the performance impact on linking time by compiling CTMark with -O3 + LTO enabled, and the result was as follows:

(unit: sec.)
			bed7884	+D77524	speedup
7zip			47.34	47.29	0.12%
bullet			9.83	9.62	2.28%
clamscan		25.42	25.44	-0.06%
SPASS			19.91	19.93	-0.12%
consumer-typeset	19.12	19.24	-0.64%
kimwitu++		15.19	15.06	0.88%
lencod			39.91	40.03	-0.30%
mafft			8.77	8.77	0.05%
sqlite3			25.07	25.33	-1.02%
tramp3d-v4		36.23	36.21	0.06%

The geometric mean of speedup is -0.13%, which seems to be within an error, considering that compilation time (without linking time) showed 0.1% average speedup which should be 0% in theory because LTO is enabled.

As the validity of this patch has been discussed at D77523, I think it is okay to add the pass before LSR.

[nikic]

LGTM

[aqjune]

Thanks!

[sanwou01]

@aqjune thanks again for working on this pass. I can confirm that the regression in mcf we originally reported on D76483 is now resolved!

[aqjune]

Glad to hear that! :)

[nickdesaulniers]

https://bugs.llvm.org/show_bug.cgi?id=50440 was filed as a response to this. Since it doesn't look like we'll be able to fix forward quickly and clang 12.0.1 is about to release, I've filed https://bugs.llvm.org/show_bug.cgi?id=50573 to revert this from the main and release/12.x branches.

llvm/lib/CodeGen/TargetPassConfig.cpp

 //===- TargetPassConfig.cpp - Target independent code generation passes ---===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
void TargetPassConfig::addIRPasses() {
 
   // Before running any passes, run the verifier to determine if the input
   // coming from the front-end and/or optimizer is valid.
   if (!DisableVerify)
     addPass(createVerifierPass());
 
   // Run loop strength reduction before anything else.
   if (getOptLevel() != CodeGenOpt::None && !DisableLSR) {
+    addPass(createCanonicalizeFreezeInLoopsPass());
     addPass(createLoopStrengthReducePass());
     if (PrintLSR)
       addPass(createPrintFunctionPass(dbgs(), "\n\n*** Code after LSR ***\n"));
   }
 
   if (getOptLevel() != CodeGenOpt::None) {
     // The MergeICmpsPass tries to create memcmp calls by grouping sequences of
     // loads and compares. ExpandMemCmpPass then tries to expand those calls