GlobalISel: Add known bits to InstructionSelector

[arsenm]

Summary

AMDGPU uses this for some addressing mode selection patterns. The
analysis run itself doesn't do anything so it seems easier to just
always require this than adding a way to opt in.

[aditya_nandakumar]

Hi Matt, this looks reasonable - though when (if) we add a more complex implementation (perhaps precomputes knownbits) we might have to revisit this.
For now, it's close to zero cost. LGTM.

[arsenm]

r370388

[peter.smith]

This change has caused some test failures in our Armv7 only builders http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-full-sh and http://lab.llvm.org:8011/builders/clang-cmake-armv7-full . An example build failure is http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/7366

All the errors are in global isel tests and have the form:

Pass 'InstructionSelect' is not initialized.
Verify if there is a pass dependency cycle.
Required Passes:
	Target Pass Configuration

It is reproducible on an X86_64 machine, although only when -DLLVM_TARGETS_TO_BUILD="ARM" and only ARM (the bot configuration). If I include AArch64 as well as ARM then the tests pass. I presume this change has added a pass dependency that is not being initialised in the ARM target, but it does if there is another Target that does initialize it. I note that the ARM equivalent of AArch64InstructionSelector doesn't call have the equivalent of setupMF.

Can you take a look?

[arsenm]

I note that the ARM equivalent of AArch64InstructionSelector doesn't call have the equivalent of setupMF.

That isn't it. InstructionSelector isn't the pass itself (InstructionSelect is), and setupMF has a default implementation

[peter.smith]

In D66921#1652690, @arsenm wrote:

I note that the ARM equivalent of AArch64InstructionSelector doesn't call have the equivalent of setupMF.

That isn't it. InstructionSelector isn't the pass itself (InstructionSelect is), and setupMF has a default implementation

Agreed. I arrived at this patch via bisection. I've done a few more experiments and it seems like it is not just the ARM Global Isel tests that depend on the AArch64 backend. If I build the X86 and AMDGPU targets without AArch64 then some of their Global Isel tests fail aswell. Last bit of naive speculation before I go home. The only place in the codebase with INITIALIZE_PASS_DEPENDENCY(GISelKnownBitsAnalysis) is in AArch64PreLegalizerCombiner.cpp. Does this need to be added to the other backends that use Global Isel?

[arsenm]

In D66921#1652802, @peter.smith wrote:

In D66921#1652690, @arsenm wrote:

I note that the ARM equivalent of AArch64InstructionSelector doesn't call have the equivalent of setupMF.

That isn't it. InstructionSelector isn't the pass itself (InstructionSelect is), and setupMF has a default implementation

Agreed. I arrived at this patch via bisection. I've done a few more experiments and it seems like it is not just the ARM Global Isel tests that depend on the AArch64 backend. If I build the X86 and AMDGPU targets without AArch64 then some of their Global Isel tests fail aswell. Last bit of naive speculation before I go home. The only place in the codebase with INITIALIZE_PASS_DEPENDENCY(GISelKnownBitsAnalysis) is in AArch64PreLegalizerCombiner.cpp. Does this need to be added to the other backends that use Global Isel?

This is missing from InstructionSelect

[dyung]

Is someone working on a fix for this that will go in soon? I am about to revert this change because it is blocking our internal builds as well as breaking both of the public PS4 bots.

[arsenm]

In D66921#1652859, @dyung wrote:

Is someone working on a fix for this that will go in soon? I am about to revert this change because it is blocking our internal builds as well as breaking both of the public PS4 bots.

I have a patch I'm waiting for tests to commit

include/llvm/CodeGen/GlobalISel/InstructionSelector.h

 #include <functional>
 #include <initializer_list>
 #include <vector>
 
 namespace llvm {
 
 class APInt;
 class APFloat;
+class GISelKnownBits;
 class MachineInstr;
 class MachineInstrBuilder;
 class MachineFunction;
 class MachineOperand;
 class MachineRegisterInfo;
 class RegisterBankInfo;
 class TargetInstrInfo;
 class TargetRegisterClass;
public:
   /// \pre  I.getParent() && I.getParent()->getParent()
   /// \post
   ///   if returns true:
   ///     for I in all mutated/inserted instructions:
   ///       !isPreISelGenericOpcode(I.getOpcode())
   virtual bool select(MachineInstr &I) = 0;
 
   CodeGenCoverage *CoverageInfo = nullptr;
+  GISelKnownBits *KnownBits = nullptr;
   MachineFunction *MF = nullptr;
 
   /// Setup per-MF selector state.
-  virtual void setupMF(MachineFunction &mf, CodeGenCoverage &covinfo) {
+  virtual void setupMF(MachineFunction &mf,
+                       GISelKnownBits &KB,
+                       CodeGenCoverage &covinfo) {
     CoverageInfo = &covinfo;
+    KnownBits = &KB;
     MF = &mf;
   }
 
 protected:
   using ComplexRendererFns =
       Optional<SmallVector<std::function<void(MachineInstrBuilder &)>, 4>>;
   using RecordedMIVector = SmallVector<MachineInstr *, 4>;
   using NewMIVector = SmallVector<MachineInstrBuilder, 4>;

lib/CodeGen/GlobalISel/InstructionSelect.cpp

 //===- llvm/CodeGen/GlobalISel/InstructionSelect.cpp - InstructionSelect ---==//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 /// \file
 /// This file implements the InstructionSelect class.
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/GlobalISel/InstructionSelect.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/CodeGen/GlobalISel/GISelKnownBits.h"
 #include "llvm/CodeGen/GlobalISel/InstructionSelector.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"

 INITIALIZE_PASS_END(InstructionSelect, DEBUG_TYPE,
                     "Select target instructions out of generic instructions",
                     false, false)
 
 InstructionSelect::InstructionSelect() : MachineFunctionPass(ID) { }
 
 void InstructionSelect::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<TargetPassConfig>();
+  AU.addRequired<GISelKnownBitsAnalysis>();
+  AU.addPreserved<GISelKnownBitsAnalysis>();
   getSelectionDAGFallbackAnalysisUsage(AU);
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
 bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) {
   // If the ISel pipeline failed, do not bother running that pass.
   if (MF.getProperties().hasProperty(
           MachineFunctionProperties::Property::FailedISel))
     return false;
 
   LLVM_DEBUG(dbgs() << "Selecting function: " << MF.getName() << '\n');
+  GISelKnownBits &KB = getAnalysis<GISelKnownBitsAnalysis>().get(MF);
 
   const TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
   InstructionSelector *ISel = MF.getSubtarget().getInstructionSelector();
   CodeGenCoverage CoverageInfo;
   assert(ISel && "Cannot work without InstructionSelector");
-  ISel->setupMF(MF, CoverageInfo);
+  ISel->setupMF(MF, KB, CoverageInfo);
 
   // An optimization remark emitter. Used to report failures.
   MachineOptimizationRemarkEmitter MORE(MF, /*MBFI=*/nullptr);
 
   // FIXME: There are many other MF/MFI fields we need to initialize.
 
   MachineRegisterInfo &MRI = MF.getRegInfo();
 #ifndef NDEBUG

lib/Target/AArch64/AArch64InstructionSelector.cpp

 public:
   AArch64InstructionSelector(const AArch64TargetMachine &TM,
                              const AArch64Subtarget &STI,
                              const AArch64RegisterBankInfo &RBI);
 
   bool select(MachineInstr &I) override;
   static const char *getName() { return DEBUG_TYPE; }
 
-  void setupMF(MachineFunction &MF, CodeGenCoverage &CoverageInfo) override {
-    InstructionSelector::setupMF(MF, CoverageInfo);
+  void setupMF(MachineFunction &MF, GISelKnownBits &KB,
+               CodeGenCoverage &CoverageInfo) override {
+    InstructionSelector::setupMF(MF, KB, CoverageInfo);
 
     // hasFnAttribute() is expensive to call on every BRCOND selection, so
     // cache it here for each run of the selector.
     ProduceNonFlagSettingCondBr =
         !MF.getFunction().hasFnAttribute(Attribute::SpeculativeLoadHardening);
   }
 
 private:

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

 ; 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
 ; 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
 
 ; DISABLED-NOT: IRTranslator
 
 ; DISABLED: AArch64 Instruction Selection
 ; DISABLED: Finalize ISel and expand pseudo-instructions
 
 define void @empty() {
   ret void
 }

test/CodeGen/AArch64/O0-pipeline.ll

 ; CHECK-NEXT:       Analysis containing CSE Info
 ; CHECK-NEXT:       IRTranslator
 ; CHECK-NEXT:       Analysis for ComputingKnownBits
 ; CHECK-NEXT:       AArch64PreLegalizerCombiner
 ; CHECK-NEXT:       Analysis containing CSE Info
 ; CHECK-NEXT:       Legalizer
 ; CHECK-NEXT:       RegBankSelect
 ; CHECK-NEXT:       Localizer
+; CHECK-NEXT:       Analysis for ComputingKnownBits
 ; CHECK-NEXT:       InstructionSelect
 ; CHECK-NEXT:       ResetMachineFunction
 ; CHECK-NEXT:       AArch64 Instruction Selection
 ; CHECK-NEXT:       Finalize ISel and expand pseudo-instructions
 ; CHECK-NEXT:       Local Stack Slot Allocation
 ; CHECK-NEXT:       Eliminate PHI nodes for register allocation
 ; CHECK-NEXT:       Two-Address instruction pass
 ; CHECK-NEXT:       Fast Register Allocator