[GlobalISel][X86] support G_FRAME_INDEX instruction selection.

[igorb]

Summary

Support G_FRAME_INDEX instruction selection.

[ab]

Seems reasonable; a couple nits.

test/CodeGen/X86/GlobalISel/frameIndex-instructionselect.mir

3

Also test X32 LEA64_32r? and rename the i386 check prefix ;)

7

FWIW, I found this function more confusing than helpful: it doesn't involve any frame index for x86_64, yet the mir has one. Maybe make the function an empty void() ? We still want to teach mir not to require an IR definition someday.

Alternatively, write a test using an alloca? That should have the same behavior everywhere.

[igorb]

Thanks for the comments.

[ab]

LGTM, thanks!
Looks like the test has a file mode svn prop, maybe you can undo that?

lib/Target/X86/X86LegalizerInfo.cpp

72

Maybe check STI->isTarget64BitILP32 so that you don't have to carry around a TargetMachine?

[igorb]

Thanks for the review!
Closed by commit rL298800

lib/Target/X86/X86InstructionSelector.h

private:
   unsigned getSubOp(LLT &Ty, const RegisterBank &RB) const;
   unsigned getLoadStoreOp(LLT &Ty, const RegisterBank &RB, unsigned Opc,
                           uint64_t Alignment) const;
 
   bool selectBinaryOp(MachineInstr &I, MachineRegisterInfo &MRI,
                       MachineFunction &MF) const;
   bool selectLoadStoreOp(MachineInstr &I, MachineRegisterInfo &MRI,
                          MachineFunction &MF) const;
+  bool selectFrameIndex(MachineInstr &I, MachineRegisterInfo &MRI,
+                        MachineFunction &MF) const;
 
   const X86Subtarget &STI;
   const X86InstrInfo &TII;
   const X86RegisterInfo &TRI;
   const X86RegisterBankInfo &RBI;
 
 #define GET_GLOBALISEL_TEMPORARIES_DECL
 #include "X86GenGlobalISel.inc"
 #undef GET_GLOBALISEL_TEMPORARIES_DECL
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_X86_X86INSTRUCTIONSELECTOR_H
 #endif // LLVM_BUILD_GLOBAL_ISEL

lib/Target/X86/X86InstructionSelector.cpp

assert(I.getNumOperands() == I.getNumExplicitOperands() &&
          "Generic instruction has unexpected implicit operands\n");
 
   // TODO: This should be implemented by tblgen, pattern with predicate not
   // supported yet.
   if (selectBinaryOp(I, MRI, MF))
     return true;
   if (selectLoadStoreOp(I, MRI, MF))
     return true;
+  if (selectFrameIndex(I, MRI, MF))
+    return true;
 
   return selectImpl(I);
 }
 
 unsigned X86InstructionSelector::getFAddOp(LLT &Ty,
                                            const RegisterBank &RB) const {
 
   if (X86::VECRRegBankID != RB.getID())
bool X86InstructionSelector::selectLoadStoreOp(MachineInstr &I,
   else {
     // G_STORE (VAL, Addr), X86Store instruction (Addr, VAL)
     I.RemoveOperand(0);
     addOffset(MIB, 0).addUse(DefReg);
   }
   return constrainSelectedInstRegOperands(I, TII, TRI, RBI);
 }
 
+bool X86InstructionSelector::selectFrameIndex(MachineInstr &I,
+                                              MachineRegisterInfo &MRI,
+                                              MachineFunction &MF) const {
+  if (I.getOpcode() != TargetOpcode::G_FRAME_INDEX)
+    return false;
+
+  const unsigned DefReg = I.getOperand(0).getReg();
+  LLT Ty = MRI.getType(DefReg);
+
+  // Use LEA to calculate frame index.
+  unsigned NewOpc;
+  if (Ty == LLT::pointer(0, 64))
+    NewOpc = X86::LEA64r;
+  else if (Ty == LLT::pointer(0, 32))
+    NewOpc = STI.isTarget64BitILP32() ? X86::LEA64_32r : X86::LEA32r;
+  else
+    llvm_unreachable("Can't select G_FRAME_INDEX, unsupported type.");
+
+  I.setDesc(TII.get(NewOpc));
+  MachineInstrBuilder MIB(MF, I);
+  addOffset(MIB, 0);
+
+  return constrainSelectedInstRegOperands(I, TII, TRI, RBI);
+}

lib/Target/X86/X86LegalizerInfo.h

 #ifndef LLVM_LIB_TARGET_X86_X86MACHINELEGALIZER_H
 #define LLVM_LIB_TARGET_X86_X86MACHINELEGALIZER_H
 
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
 
 namespace llvm {
 
 class X86Subtarget;
+class X86TargetMachine;
 
 /// This class provides the information for the target register banks.
 class X86LegalizerInfo : public LegalizerInfo {
 private:
   /// Keep a reference to the X86Subtarget around so that we can
   /// make the right decision when generating code for different targets.
   const X86Subtarget &Subtarget;
+  const X86TargetMachine &TM;
 
 public:
-  X86LegalizerInfo(const X86Subtarget &STI);
+  X86LegalizerInfo(const X86Subtarget &STI, const X86TargetMachine &TM);
 
 private:
   void setLegalizerInfo32bit();
   void setLegalizerInfo64bit();
   void setLegalizerInfoSSE1();
   void setLegalizerInfoSSE2();
 };
-} // End llvm namespace.
+} // namespace llvm
 #endif

lib/Target/X86/X86LegalizerInfo.cpp

 //===- X86LegalizerInfo.cpp --------------------------------------*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 /// \file
 /// This file implements the targeting of the Machinelegalizer class for X86.
 /// \todo This should be generated by TableGen.
 //===----------------------------------------------------------------------===//
 
 #include "X86LegalizerInfo.h"
 #include "X86Subtarget.h"
+#include "X86TargetMachine.h"
 #include "llvm/CodeGen/ValueTypes.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Target/TargetOpcodes.h"
 
 using namespace llvm;
 using namespace TargetOpcode;
 
 #ifndef LLVM_BUILD_GLOBAL_ISEL
 #error "You shouldn't build this"
 #endif
 
-X86LegalizerInfo::X86LegalizerInfo(const X86Subtarget &STI) : Subtarget(STI) {
+X86LegalizerInfo::X86LegalizerInfo(const X86Subtarget &STI,
+                                   const X86TargetMachine &TM)
+    : Subtarget(STI), TM(TM) {
 
   setLegalizerInfo32bit();
   setLegalizerInfo64bit();
   setLegalizerInfoSSE1();
   setLegalizerInfoSSE2();
 
   computeTables();
 }
void X86LegalizerInfo::setLegalizerInfo32bit() {
 
   for (unsigned MemOp : {G_LOAD, G_STORE}) {
     for (auto Ty : {s8, s16, s32, p0})
       setAction({MemOp, Ty}, Legal);
 
     // And everything's fine in addrspace 0.
     setAction({MemOp, 1, p0}, Legal);
   }
+
+  // Pointer-handling
+  setAction({G_FRAME_INDEX, p0}, Legal);
 }
 
 void X86LegalizerInfo::setLegalizerInfo64bit() {
 
   if (!Subtarget.is64Bit())
     return;
 
-  const LLT p0 = LLT::pointer(0, 64);
+  const LLT p0 = LLT::pointer(0, TM.getPointerSize() * 8);
   const LLT s8 = LLT::scalar(8);
   const LLT s16 = LLT::scalar(16);
   const LLT s32 = LLT::scalar(32);
   const LLT s64 = LLT::scalar(64);
 
   for (unsigned BinOp : {G_ADD, G_SUB})
     for (auto Ty : {s8, s16, s32, s64})
       setAction({BinOp, Ty}, Legal);
 
   for (unsigned MemOp : {G_LOAD, G_STORE}) {
     for (auto Ty : {s8, s16, s32, s64, p0})
       setAction({MemOp, Ty}, Legal);
 
     // And everything's fine in addrspace 0.
     setAction({MemOp, 1, p0}, Legal);
   }
+
+  // Pointer-handling
+  setAction({G_FRAME_INDEX, p0}, Legal);
 }
 
 void X86LegalizerInfo::setLegalizerInfoSSE1() {
   if (!Subtarget.hasSSE1())
     return;
 
   const LLT s32 = LLT::scalar(32);
   const LLT v4s32 = LLT::vector(4, 32);
void X86LegalizerInfo::setLegalizerInfoSSE2() {
 
   for (unsigned BinOp : {G_FADD, G_FSUB, G_FMUL, G_FDIV})
     for (auto Ty : {s64, v2s64})
       setAction({BinOp, Ty}, Legal);
 
   for (unsigned BinOp : {G_ADD, G_SUB})
     for (auto Ty : {v4s32})
       setAction({BinOp, Ty}, Legal);
-
 }

lib/Target/X86/X86TargetMachine.cpp

if (!I) {
     I = llvm::make_unique<X86Subtarget>(TargetTriple, CPU, FS, *this,
                                         Options.StackAlignmentOverride);
 #ifndef LLVM_BUILD_GLOBAL_ISEL
     GISelAccessor *GISel = new GISelAccessor();
 #else
     X86GISelActualAccessor *GISel = new X86GISelActualAccessor();
 
     GISel->CallLoweringInfo.reset(new X86CallLowering(*I->getTargetLowering()));
-    GISel->Legalizer.reset(new X86LegalizerInfo(*I));
+    GISel->Legalizer.reset(new X86LegalizerInfo(*I, *this));
 
     auto *RBI = new X86RegisterBankInfo(*I->getRegisterInfo());
     GISel->RegBankInfo.reset(RBI);
     GISel->InstSelector.reset(new X86InstructionSelector(*I, *RBI));
 
 #endif
     I->setGISelAccessor(*GISel);
   }

test/CodeGen/X86/GlobalISel/frameIndex-instructionselect.mir

+# RUN: llc -mtriple=x86_64-linux-gnu    -global-isel -run-pass=instruction-select %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=X64
+# RUN: llc -mtriple=i386-linux-gnu      -global-isel -run-pass=instruction-select %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=X32
+# RUN: llc -mtriple=x86_64-linux-gnux32 -global-isel -run-pass=instruction-select %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=X32ABI
+
+--- |
+  define i32* @allocai32() {
+    %ptr1 = alloca i32
+    ret i32* %ptr1
+  }
+
+...
+---
+name:            allocai32
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK-LABEL: name:            allocai32
+# CHECK: registers:
+# CHECK-X32:     - { id: 0, class: gr32 }
+# CHECK-X32ABI:  - { id: 0, class: gr32 }
+# CHECK-X64:     - { id: 0, class: gr64 }
+registers:       
+  - { id: 0, class: gpr }
+stack:           
+  - { id: 0, name: ptr1, offset: 0, size: 4, alignment: 4 }
+
+# CHECK-X32:    %0 = LEA32r %stack.0.ptr1, 1, _, 0, _
+# CHECK-X32ABI: %0 = LEA64_32r %stack.0.ptr1, 1, _, 0, _
+# CHECK-X64:    %0 = LEA64r %stack.0.ptr1, 1, _, 0, _     
+body:             |
+  bb.1 (%ir-block.0):
+    %0(p0) = G_FRAME_INDEX %stack.0.ptr1
+    %eax = COPY %0(p0)
+    RET 0, implicit %eax
+
+...

test/CodeGen/X86/GlobalISel/frameIndex.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=x86_64-linux-gnu    -global-isel < %s -o - | FileCheck %s --check-prefix=X64
+; RUN: llc -mtriple=x86_64-linux-gnu                 < %s -o - | FileCheck %s --check-prefix=X64
+; RUN: llc -mtriple=i386-linux-gnu      -global-isel < %s -o - | FileCheck %s --check-prefix=X32
+; RUN: llc -mtriple=i386-linux-gnu                   < %s -o - | FileCheck %s --check-prefix=X32
+; RUN: llc -mtriple=x86_64-linux-gnux32 -global-isel < %s -o - | FileCheck %s --check-prefix=X32ABI
+; RUN: llc -mtriple=x86_64-linux-gnux32              < %s -o - | FileCheck %s --check-prefix=X32ABI
+
+define i32* @allocai32() {
+; X64-LABEL: allocai32:
+; X64:       # BB#0:
+; X64-NEXT:    leaq -4(%rsp), %rax
+; X64-NEXT:    retq
+;
+; X32-LABEL: allocai32:
+; X32:       # BB#0:
+; X32-NEXT:    pushl %eax
+; X32-NEXT:  .Lcfi0:
+; X32-NEXT:    .cfi_def_cfa_offset 8
+; X32-NEXT:    movl %esp, %eax
+; X32-NEXT:    popl %ecx
+; X32-NEXT:    retl
+;
+; X32ABI-LABEL: allocai32:
+; X32ABI:       # BB#0:
+; X32ABI-NEXT:    leal -4(%rsp), %eax
+; X32ABI-NEXT:    retq
+  %ptr1 = alloca i32
+  ret i32* %ptr1
+}