[AArch64][GlobalISel] Enable the localizer for optimized builds

[aemerson]

Summary

Despite the fact that the localizer's original motivation was to fix horrendous constant spilling at -O0, shortening live ranges still has net benefits even with optimizations enabled.

On an -Os build of CTMark, doing this improves code size by 0.5% geomean.

There are a few regressions, bullet increasing in size by 0.5%. One example from bullet where code size increased slightly was due to GlobalISel actually now generating the same code as SelectionDAG. So we actually have an opportunity in future to implement better heuristics for localization and therefore be *better* than SDAG in some cases. In relation to other optimizations though that one is relatively minor.

[paquette]

LGTM

[qcolombet]

Hi Amara,

Where are the benefits coming from for optimized build?
I am guessing less copies/spills and in that case, I think we should try to fix the allocator, but that's more a long term plan.
Fill a PR though with an example so that we don't forget.

LGTM.

Cheers,
-Quentin

[aemerson]

In D67303#1661664, @qcolombet wrote:

Hi Amara,

Where are the benefits coming from for optimized build?
I am guessing less copies/spills and in that case, I think we should try to fix the allocator, but that's more a long term plan.
Fill a PR though with an example so that we don't forget.

LGTM.

Cheers,
-Quentin

The swifterror test change in this patch is one example. Live ranges over function calls can be problematic if under register pressure, which can cause additional moves, if not spills. There are others of course. The greedy allocator just doesn't seem to expect long live ranges across basic-block boundaries.

[aemerson]

Filed https://bugs.llvm.org/show_bug.cgi?id=43247 for the regalloc issue.

[qcolombet]

The swifterror test change in this patch is one example. Live ranges over function calls can be problematic if under register pressure, which can cause additional moves, if not spills. There are others of course

Is it happening in this specific test (more moves or spill)?
I'd like to have something to reproduce the issue.

llvm/lib/Target/AArch64/AArch64TargetMachine.cpp

 }
 
 bool AArch64PassConfig::addRegBankSelect() {
   addPass(new RegBankSelect());
   return false;
 }
 
 void AArch64PassConfig::addPreGlobalInstructionSelect() {
-  // Workaround the deficiency of the fast register allocator.
-  if (TM->getOptLevel() == CodeGenOpt::None)
-    addPass(new Localizer());
 }
 
 bool AArch64PassConfig::addGlobalInstructionSelect() {
   addPass(new InstructionSelect());
   return false;
 }
 
 bool AArch64PassConfig::addILPOpts() {

llvm/test/CodeGen/AArch64/GlobalISel/gisel-commandline-option.ll

 ; ENABLED-NEXT:  Analysis for ComputingKnownBits
 ; ENABLED-NEXT:  PreLegalizerCombiner
 ; VERIFY-NEXT:   Verify generated machine code
 ; ENABLED-NEXT:  Analysis containing CSE Info
 ; ENABLED-NEXT:  Legalizer
 ; VERIFY-NEXT:   Verify generated machine code
 ; ENABLED-NEXT:  RegBankSelect
 ; VERIFY-NEXT:   Verify generated machine code
-; ENABLED-O0-NEXT:  Localizer
+; ENABLED-NEXT:  Localizer
 ; VERIFY-O0-NEXT:   Verify generated machine code
 ; ENABLED-NEXT: Analysis for ComputingKnownBits
 ; ENABLED-NEXT:  InstructionSelect
 ; VERIFY-NEXT:   Verify generated machine code
 ; ENABLED-NEXT:  ResetMachineFunction
 
 ; FALLBACK:       AArch64 Instruction Selection
 ; NOFALLBACK-NOT: AArch64 Instruction Selection

llvm/test/CodeGen/AArch64/GlobalISel/localizer-in-O0-pipeline.mir

 
 # First block remains untouched
 # CHECK: body
 # CHECK:   %4:fpr(s32) = G_FCONSTANT float 1.000000e+00
 # CHECK:   %5:fpr(s32) = G_FCONSTANT float 2.000000e+00
 
 # Second block will get the constant 1.0 when the localizer is enabled.
 # CHECK: bb.1.{{[a-zA-Z0-9]+}}:
-# OPT-NOT: G_FCONSTANT
+# OPT: [[FONE:%[0-9]+]]:fpr(s32) = G_FCONSTANT float 1.000000e+00
 # OPTNONE: [[FONE:%[0-9]+]]:fpr(s32) = G_FCONSTANT float 1.000000e+00
 # CHECK: G_BR %bb.3
 
 # Thrid block will get the constant 2.0 when the localizer is enabled.
 # CHECK: bb.2.{{[a-zA-Z0-9]+}}:
-# OPT-NOT: G_FCONSTANT
+# OPT: [[FTWO:%[0-9]+]]:fpr(s32) = G_FCONSTANT float 2.000000e+00
 # OPTNONE: [[FTWO:%[0-9]+]]:fpr(s32) = G_FCONSTANT float 2.000000e+00
 
 # CHECK: bb.3.end
 # OPTNONE: %2:fpr(s32) = PHI [[FONE]](s32), %bb.1, [[FTWO]](s32), %bb.2
-# OPT: %2:fpr(s32) = PHI %4(s32), %bb.1, %5(s32), %bb.2
+# OPT: %2:fpr(s32) = PHI [[FONE]](s32), %bb.1, [[FTWO]](s32), %bb.2
 # CHECK-NEXT: G_FADD %0, %2
 body:             |
   bb.0 (%ir-block.0):
     liveins: $s0, $w0
 
     %0(s32) = COPY $s0
     %6(s32) = COPY $w0
     %1(s1) = G_TRUNC %6

llvm/test/CodeGen/AArch64/GlobalISel/swifterror.ll

handler:
   ret float 1.0
 }
 
 ; "foo_if" is a function that takes a swifterror parameter, it sets swifterror
 ; under a certain condition.
 define float @foo_if(%swift_error** swifterror %error_ptr_ref, i32 %cc) {
 ; CHECK-LABEL: foo_if:
 ; CHECK: cbz w0
-; CHECK: mov [[ID:w[0-9]+]], #1
 ; CHECK: mov w0, #16
 ; CHECK: malloc
+; CHECK: mov [[ID:w[0-9]+]], #1
 ; CHECK: strb [[ID]], [x0, #8]
 ; CHECK: mov x21, x0
 ; CHECK-NOT: x21
 ; CHECK: ret
 
 entry:
   %cond = icmp ne i32 %cc, 0
   br i1 %cond, label %gen_error, label %normal