Add load/store functionality

[rkotler]

Summary

This patches allows non conversions like i1=i2; where both are global ints.
In addition, arithmetic and other things start to work since fast-isel will use
existing patterns for non fast-isel from tablegen files where applicable.

In addition i8, i16 will work in this limited context for assignment without the need
for sign extension (zero or signed). It does not matter how i8 or i16 are loaded (zero or sign extended)
since only the 8 or 16 relevant bits are used and clang will ask for sign extension before using them in
arithmetic. This is all made more complete in forthcoming patches.

for example:

int i, j=1, k=3;
 
void foo() {
  i = j + k;
}

Keep in mind that this pass is not enabled right now and is an experimental pass
It can only be enabled with a hidden option to llvm of -mips-fast-isel.

[rkotler]

• fix register class

[rkotler]

The following test case executes correctly:

~/llvmw/build/Debug+Asserts/bin/clang -target mips-linux-gnu -mips32r2 -mllvm -fast-isel -O0 -mllvm -mips-fast-isel -mllvm -fast-isel-verbose loadstore2.c -fPIC -gcc-toolchain /mips/proj/build-compiler/stable-nightly-gcc/install-mips-mti-linux-gnu -EL

/mips/arch/overflow/codesourcery/mips-linux-gnu/pro/release/2011.09-90/Linux/bin/mips-linux-gnu-qemu-el -r 2.6.38 -E LD_LIBRARY_PATH=/mips/proj/build-compiler/stable-nightly-gcc/install-mips-mti-linux-gnu/sysroot/el/usr/lib:/mips/proj/build-compiler/stable-nightly-gcc/install-mips-mti-linux-gnu/sysroot/el/usr/usr/lib:/mips/proj/build-compiler/stable-nightly-gcc/install-mips-mti-linux-gnu/mips-mti-linux-gnu/lib/el -L /mips/proj/build-compiler/stable-nightly-gcc/install-mips-mti-linux-gnu/sysroot/el/usr a.out

char c1, c2=6;
short s1, s2=123;
int i1, i2=65536;
float f1, f2=1.978;
double d1, d2=1.23456789876;

void cfoo() {

c1=c2;

}

void sfoo() {

s1=s2;

}

void ifoo() {

i1=i2;

}

void ffoo() {

f1=f2;

}

void dfoo() {

d1=d2;

}

int main() {

cfoo();
sfoo();
ifoo();
ffoo();
dfoo();
printf("%i %i %i %f %f \n", c1, s1, i1, f1, d1);

}

[rkotler]

This simple test with plus will also work now. I need to go through all the operators more in depth.

int i, j=1, k=3;

void foo() {

i = j + k;

}

int main() {

foo();
printf("%i \n", i);

}

[dsanders]

lib/Target/Mips/MipsFastISel.cpp

175–176 ↗

(On Diff #9662)

Is this if-statement correct? It looks like we are rejecting sign-extended loads here (or possibly any-extended), but below we are picking the sign-extended load instructions.

It also seems that IsZExt is always true at the moment. As far as I can tell no caller passes it in and the default value is true.

231 ↗

(On Diff #9662)

Whitespace nit around the '='.

Can you also fix the ones nearby (in another patch)?

test/CodeGen/Mips/Fast-ISel/loadstore2.ll

24–27 ↗

(On Diff #9662)

The first operand to lb/sb should be the same register. Please use a variable to check for this too like so:

; CHECK: lb $[[R0:[0-9]+]], 0(${{[0-9]+}})
; CHECK: sb $[[R0]], 0(${{[0-9]+}})

Likewise for the other tests.

Also, one thing that didn't occur to me on the earlier patches is that '; CHECK: .ent cfoo' should be '; CHECK-LABEL: .ent cfoo'. The advantage of using CHECK-LABEL is that FileCheck isn't limited to reporting a single mismatch in the file. Instead it is limited to one mismatch per CHECK-LABEL.

[rkotler]

refresing patch

[rkotler]

Executable test cases.

/home/rkotler/llvm-push/build/Debug+Asserts/bin/clang -target mips-linux-gnu -mips32r2 -mllvm -fast-isel -O2 -mllvm -mips-fast-isel -mllvm -fast-isel-verbose loadstoreu2.c -fPIC -gcc-toolchain /mips/proj/build-compiler/stable-nightly-gcc/install-mips-mti-linux-gnu -EL -save-temps -o loadstoreu2

loadstore2.c427 BDownload

loadstoreu2.c241 BDownload

[rkotler]

• clang-format

[dsanders]

Thanks for refreshing the patch to remove the noise.

This patches allows non coversions like i1=i2; where both are ints
In addition, arithmetic and other things start to work.
for example:

int i, j=1, k=3;

void foo() {
  i = j + k;
}

For a moment after reading this, I was wondering how support for load instructions could make arithmetic work :-). I understand that this patch adds support for the global variable load and that combined with the existing support for arithmetic the whole example should now work. The summary could be a lot clearer about what the patch changes compared to what begins to work as a result of the total work so far. Also, you should mention that you added support for i8 and i16 stores in this patch.

By the way, to format code examples properly in the web interface you need to put at least two spaces at the start of each line.

LGTM with a clearer commit message (that mentions the i8/i16 store change) and the EmitLoadMem vs EmitInstStore nit fixed.

lib/Target/Mips/MipsFastISel.cpp

111 ↗

(On Diff #10432)

Nit: For consistency with EmitInstStore, you need to either drop the 'Mem' here or add it to EmitInstStore.

175–176 ↗

(On Diff #9662)

This has been fixed. The IsZExt argument has been dropped and EmitLoad now uses the unsigned loads.

test/CodeGen/Mips/Fast-ISel/loadstore2.ll

24–27 ↗

(On Diff #9662)

This has been fixed

[rkotler]

• Merge branch 'master' into 1758_8a
• fix naming issue

[rkotler]

Closed by commit rL211061 (authored by @rkotler).

llvm/trunk/lib/Target/Mips/MipsFastISel.cpp

public:
   }
 
   bool TargetSelectInstruction(const Instruction *I) override;
   unsigned TargetMaterializeConstant(const Constant *C) override;
 
   bool ComputeAddress(const Value *Obj, Address &Addr);
 
 private:
+  bool EmitLoad(MVT VT, unsigned &ResultReg, Address &Addr,
+                unsigned Alignment = 0);
   bool EmitStore(MVT VT, unsigned SrcReg, Address &Addr,
                  unsigned Alignment = 0);
+  bool SelectLoad(const Instruction *I);
   bool SelectRet(const Instruction *I);
   bool SelectStore(const Instruction *I);
 
   bool isTypeLegal(Type *Ty, MVT &VT);
   bool isLoadTypeLegal(Type *Ty, MVT &VT);
 
   unsigned MaterializeFP(const ConstantFP *CFP, MVT VT);
   unsigned MaterializeGV(const GlobalValue *GV, MVT VT);
return BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc),
                    DstReg);
   }
 
   MachineInstrBuilder EmitInstStore(unsigned Opc, unsigned SrcReg,
                                     unsigned MemReg, int64_t MemOffset) {
     return EmitInst(Opc).addReg(SrcReg).addReg(MemReg).addImm(MemOffset);
   }
 
+  MachineInstrBuilder EmitInstLoad(unsigned Opc, unsigned DstReg,
+                                      unsigned MemReg, int64_t MemOffset) {
+    return EmitInst(Opc, DstReg).addReg(MemReg).addImm(MemOffset);
+  }
+
 #include "MipsGenFastISel.inc"
 };
 
 bool MipsFastISel::isTypeLegal(Type *Ty, MVT &VT) {
   EVT evt = TLI.getValueType(Ty, true);
   // Only handle simple types.
   if (evt == MVT::Other || !evt.isSimple())
     return false;
   VT = evt.getSimpleVT();
 
   // Handle all legal types, i.e. a register that will directly hold this
   // value.
   return TLI.isTypeLegal(VT);
 }
 
 bool MipsFastISel::isLoadTypeLegal(Type *Ty, MVT &VT) {
   if (isTypeLegal(Ty, VT))
     return true;
   // We will extend this in a later patch:
   //   If this is a type than can be sign or zero-extended to a basic operation
   //   go ahead and accept it now.
+  if (VT == MVT::i8 || VT == MVT::i16)
+    return true;
   return false;
 }
 
 bool MipsFastISel::ComputeAddress(const Value *Obj, Address &Addr) {
   // This construct looks a big awkward but it is how other ports handle this
   // and as this function is more fully completed, these cases which
   // return false will have additional code in them.
   //
   if (isa<Instruction>(Obj))
     return false;
   else if (isa<ConstantExpr>(Obj))
     return false;
   Addr.Base.Reg = getRegForValue(Obj);
   return Addr.Base.Reg != 0;
 }
 
+bool MipsFastISel::EmitLoad(MVT VT, unsigned &ResultReg, Address &Addr,
+                            unsigned Alignment) {
+  //
+  // more cases will be handled here in following patches.
+  //
+  unsigned Opc;
+  switch (VT.SimpleTy) {
+  case MVT::i32: {
+    ResultReg = createResultReg(&Mips::GPR32RegClass);
+    Opc = Mips::LW;
+    break;
+  }
+  case MVT::i16: {
+    ResultReg = createResultReg(&Mips::GPR32RegClass);
+    Opc = Mips::LHu;
+    break;
+  }
+  case MVT::i8: {
+    ResultReg = createResultReg(&Mips::GPR32RegClass);
+    Opc = Mips::LBu;
+    break;
+  }
+  case MVT::f32: {
+    ResultReg = createResultReg(&Mips::FGR32RegClass);
+    Opc = Mips::LWC1;
+    break;
+  }
+  case MVT::f64: {
+    ResultReg = createResultReg(&Mips::AFGR64RegClass);
+    Opc = Mips::LDC1;
+    break;
+  }
+  default:
+    return false;
+  }
+  EmitInstLoad(Opc, ResultReg, Addr.Base.Reg, Addr.Offset);
+  return true;
+}
+
 // Materialize a constant into a register, and return the register
 // number (or zero if we failed to handle it).
 unsigned MipsFastISel::TargetMaterializeConstant(const Constant *C) {
   EVT CEVT = TLI.getValueType(C->getType(), true);
 
   // Only handle simple types.
   if (!CEVT.isSimple())
     return 0;
unsigned MipsFastISel::TargetMaterializeConstant(const Constant *C) {
   return 0;
 }
 
 bool MipsFastISel::EmitStore(MVT VT, unsigned SrcReg, Address &Addr,
                              unsigned Alignment) {
   //
   // more cases will be handled here in following patches.
   //
-  if (VT == MVT::i32)
-    EmitInstStore(Mips::SW, SrcReg, Addr.Base.Reg, Addr.Offset);
-  else if (VT == MVT::f32)
-    EmitInstStore(Mips::SWC1, SrcReg, Addr.Base.Reg, Addr.Offset);
-  else if (VT == MVT::f64)
-    EmitInstStore(Mips::SDC1, SrcReg, Addr.Base.Reg, Addr.Offset);
-  else
+  unsigned Opc;
+  switch (VT.SimpleTy) {
+  case MVT::i8:
+    Opc = Mips::SB;
+    break;
+  case MVT::i16:
+    Opc = Mips::SH;
+    break;
+  case MVT::i32:
+    Opc = Mips::SW;
+    break;
+  case MVT::f32:
+    Opc = Mips::SWC1;
+    break;
+  case MVT::f64:
+    Opc = Mips::SDC1;
+    break;
+  default:
+    return false;
+  }
+  EmitInstStore(Opc, SrcReg, Addr.Base.Reg, Addr.Offset);
+  return true;
+}
+
+bool MipsFastISel::SelectLoad(const Instruction *I) {
+  // Atomic loads need special handling.
+  if (cast<LoadInst>(I)->isAtomic())
+    return false;
+
+  // Verify we have a legal type before going any further.
+  MVT VT;
+  if (!isLoadTypeLegal(I->getType(), VT))
+    return false;
+
+  // See if we can handle this address.
+  Address Addr;
+  if (!ComputeAddress(I->getOperand(0), Addr))
+    return false;
+
+  unsigned ResultReg;
+  if (!EmitLoad(VT, ResultReg, Addr, cast<LoadInst>(I)->getAlignment()))
     return false;
+  UpdateValueMap(I, ResultReg);
   return true;
 }
 
 bool MipsFastISel::SelectStore(const Instruction *I) {
   Value *Op0 = I->getOperand(0);
   unsigned SrcReg = 0;
 
   // Atomic stores need special handling.

 }
 
 bool MipsFastISel::TargetSelectInstruction(const Instruction *I) {
   if (!TargetSupported)
     return false;
   switch (I->getOpcode()) {
   default:
     break;
+  case Instruction::Load:
+    return SelectLoad(I);
   case Instruction::Store:
     return SelectStore(I);
   case Instruction::Ret:
     return SelectRet(I);
   }
   return false;
 }
 }

llvm/trunk/test/CodeGen/Mips/Fast-ISel/loadstore2.ll

+; ModuleID = 'loadstore2.c'
+target datalayout = "E-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
+target triple = "mips--linux-gnu"
+
+@c2 = common global i8 0, align 1
+@c1 = common global i8 0, align 1
+; RUN: llc -march=mipsel -relocation-model=pic -O0 -mips-fast-isel -fast-isel-abort -mcpu=mips32r2 \
+; RUN:     < %s | FileCheck %s
+
+@s2 = common global i16 0, align 2
+@s1 = common global i16 0, align 2
+@i2 = common global i32 0, align 4
+@i1 = common global i32 0, align 4
+@f2 = common global float 0.000000e+00, align 4
+@f1 = common global float 0.000000e+00, align 4
+@d2 = common global double 0.000000e+00, align 8
+@d1 = common global double 0.000000e+00, align 8
+
+; Function Attrs: nounwind
+define void @cfoo() #0 {
+entry:
+  %0 = load i8* @c2, align 1
+  store i8 %0, i8* @c1, align 1
+; CHECK-LABEL:  cfoo:
+; CHECK:  lbu $[[REGc:[0-9]+]], 0(${{[0-9]+}})
+; CHECK:  sb  $[[REGc]], 0(${{[0-9]+}})
+
+
+  ret void
+}
+
+; Function Attrs: nounwind
+define void @sfoo() #0 {
+entry:
+  %0 = load i16* @s2, align 2
+  store i16 %0, i16* @s1, align 2
+; CHECK-LABEL:  sfoo:
+; CHECK:  lhu $[[REGs:[0-9]+]], 0(${{[0-9]+}})
+; CHECK:  sh  $[[REGs]], 0(${{[0-9]+}})
+
+  ret void
+}
+
+; Function Attrs: nounwind
+define void @ifoo() #0 {
+entry:
+  %0 = load i32* @i2, align 4
+  store i32 %0, i32* @i1, align 4
+; CHECK-LABEL:  ifoo:
+; CHECK:  lw  $[[REGi:[0-9]+]], 0(${{[0-9]+}})
+; CHECK:  sw  $[[REGi]], 0(${{[0-9]+}})
+
+  ret void
+}
+
+; Function Attrs: nounwind
+define void @ffoo() #0 {
+entry:
+  %0 = load float* @f2, align 4
+  store float %0, float* @f1, align 4
+; CHECK-LABEL:  ffoo:
+; CHECK:  lwc1  $f[[REGf:[0-9]+]], 0(${{[0-9]+}})
+; CHECK:  swc1  $f[[REGf]], 0(${{[0-9]+}})
+
+
+  ret void
+}
+
+; Function Attrs: nounwind
+define void @dfoo() #0 {
+entry:
+  %0 = load double* @d2, align 8
+  store double %0, double* @d1, align 8
+; CHECK-LABEL:        dfoo:
+; CHECK:        ldc1    $f[[REGd:[0-9]+]], 0(${{[0-9]+}})
+; CHECK:        sdc1    $f[[REGd]], 0(${{[0-9]+}})
+; CHECK:        .end    dfoo
+  ret void
+}
+
+attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+