[PATCH] D150179: [GlobalISel] Hide hoisted constants behind G_BITCAST to prevent folding.

[Amara Emerson via Phabricator via llvm-commits]

aemerson created this revision.
aemerson added reviewers: paquette, arsenm, foad, tschuett, Pierre-vh.
aemerson added a project: LLVM.
Herald added subscribers: StephenFan, hiraditya, kristof.beyls.
Herald added a project: All.
aemerson requested review of this revision.
Herald added a subscriber: wdng.

The constant hoisting pass tries to hoist large constants into predecessors and also
generates remat instructions in terms of the hoisted constants. These aim to prevent
codegen from rematerializing expensive constants multiple times. So we can re-use
this optimization, we can preserve the no-op bitcasts that are used to anchor
constants to the predecessor blocks.

SelectionDAG achieves this by having the OpaqueConstant node, which is just a
normal constant with an opaque flag set. I've opted to avoid introducing a new
constant generic instruction here.

However, once we've run all the combiners we can eliminate these G_BITCASTS
to allow selection to see them. For AArch64 it makes sense to do it at post-legalizer
combining, but *after* the actual combiner loop runs since we don't want it eliminated
before any constant folding runs.

This change by itself has very minor improvements in -Os CTMark overall. What this
does allow is better optimizations when future combines are added that rely on having
expensive constants remain unfolded.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150179

Files:
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll


Index: llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-hoisted-constants.ll
@@ -0,0 +1,35 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
+; RUN: llc -mtriple=aarch64-apple-ios -global-isel -stop-after=irtranslator %s -o - | FileCheck %s --check-prefix=TRANSLATED
+; RUN: llc -mtriple=aarch64-apple-ios -global-isel -stop-after=aarch64-postlegalizer-combiner %s -o - | FileCheck %s --check-prefix=PRESELECTION
+
+; Check we don't elide no-op bitcasts of constants since they're used by constant
+; hoisting to prevent constant folding/propagation.
+
+define i32 @test(i32 %a) {
+  ; TRANSLATED-LABEL: name: test
+  ; TRANSLATED: bb.1.entry:
+  ; TRANSLATED-NEXT:   liveins: $w0
+  ; TRANSLATED-NEXT: {{  $}}
+  ; TRANSLATED-NEXT:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; TRANSLATED-NEXT:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 100000
+  ; TRANSLATED-NEXT:   [[BITCAST:%[0-9]+]]:_(s32) = G_BITCAST [[C]](s32)
+  ; TRANSLATED-NEXT:   [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[BITCAST]]
+  ; TRANSLATED-NEXT:   $w0 = COPY [[ADD]](s32)
+  ; TRANSLATED-NEXT:   RET_ReallyLR implicit $w0
+  ; PRESELECTION-LABEL: name: test
+  ; PRESELECTION: bb.1.entry:
+  ; PRESELECTION-NEXT:   liveins: $w0
+  ; PRESELECTION-NEXT: {{  $}}
+  ; PRESELECTION-NEXT:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; PRESELECTION-NEXT:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 100000
+  ; PRESELECTION-NEXT:   [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[C]]
+  ; PRESELECTION-NEXT:   $w0 = COPY [[ADD]](s32)
+  ; PRESELECTION-NEXT:   RET_ReallyLR implicit $w0
+entry:
+  %hc = bitcast i32 100000 to i32
+  br label %cont
+cont:
+  %add = add i32 %a, %hc
+  ret i32 %add
+}
+
Index: llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
===================================================================
--- llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
@@ -20,6 +20,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "AArch64TargetMachine.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/CodeGen/GlobalISel/CSEInfo.h"
 #include "llvm/CodeGen/GlobalISel/Combiner.h"
 #include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
@@ -431,7 +432,27 @@
       getAnalysis<GISelCSEAnalysisWrapperPass>().getCSEWrapper();
   auto *CSEInfo = &Wrapper.get(TPC->getCSEConfig());
   Combiner C(PCInfo, TPC);
-  return C.combineMachineInstrs(MF, CSEInfo);
+  bool Changed = C.combineMachineInstrs(MF, CSEInfo);
+
+  auto &MRI = MF.getRegInfo();
+  // Eliminate any no-op bitcasts that we created to preserve hoisted constants.
+  // We wait until all the combiners have run otherwise the constants may get
+  // re-folded with others during the main combiner loop.
+  for (auto &MBB : MF) {
+    for (auto &MI : make_early_inc_range(MBB)) {
+      if (MI.getOpcode() == TargetOpcode::G_BITCAST) {
+        Register Dst = MI.getOperand(0).getReg();
+        Register Src = MI.getOperand(1).getReg();
+        if (MRI.getType(Dst) != MRI.getType(Src))
+          continue;
+        MRI.replaceRegWith(Dst, Src);
+        MI.eraseFromParent();
+        Changed = true;
+      }
+    }
+  }
+
+  return Changed;
 }
 
 char AArch64PostLegalizerCombiner::ID = 0;
Index: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -1468,8 +1468,13 @@
                                     MachineIRBuilder &MIRBuilder) {
   // If we're bitcasting to the source type, we can reuse the source vreg.
   if (getLLTForType(*U.getOperand(0)->getType(), *DL) ==
-      getLLTForType(*U.getType(), *DL))
+      getLLTForType(*U.getType(), *DL)) {
+    // If the source is a ConstantInt then it was probably created by
+    // ConstantHoisting and we should leave it alone.
+    if (isa<ConstantInt>(U.getOperand(0)))
+      return translateCast(TargetOpcode::G_BITCAST, U, MIRBuilder);
     return translateCopy(U, *U.getOperand(0), MIRBuilder);
+  }
 
   return translateCast(TargetOpcode::G_BITCAST, U, MIRBuilder);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150179.520612.patch
Type: text/x-patch
Size: 4368 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230509/9161568a/attachment.bin>