Skip to content

S
swiftshader
Commit b9f27229 by Karl Schimpf
Options

Fixes LDR and STR instructions. Two types of mistakes were being made.

First, the width was not being correctly defined for non-vector instructions. Second, the order of the width/condition was incorrect when the instruction was prefixed with a V. That is, for V prefixed instructions, the order is predicate/width while for non-V prefixed instructions the order is width/predicate. Also fixes bug in target lowering that did not always convert results of a compare to i1. BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4334 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1415953007 .
parent c59288b3
Showing with 66 additions and 44 deletions
src/IceAssemblerARM32.cpp
......@@ -705,6 +705,7 @@ void AssemblerARM32::ldr(const Operand *OpRt, const Operand *OpAddress,
// ldr<c> <Rt>, [<Rn>{, #+/-<imm12>}] ; p=1, w=0
// ldr<c> <Rt>, [<Rn>], #+/-<imm12> ; p=1, w=1
// ldr<c> <Rt>, [<Rn>, #+/-<imm12>]! ; p=0, w=1
//
// LDRB (immediate) - ARM section A8.8.68, encoding A1:
// ldrb<c> <Rt>, [<Rn>{, #+/-<imm12>}] ; p=1, w=0
// ldrb<c> <Rt>, [<Rn>], #+/-<imm12> ; p=1, w=1
......@@ -716,7 +717,7 @@ void AssemblerARM32::ldr(const Operand *OpRt, const Operand *OpAddress,
const Type Ty = OpRt->getType();
if (!(Ty == IceType_i32 || Ty == IceType_i8)) // TODO(kschimpf) Expand?
return setNeedsTextFixup();
const bool IsByte = typeWidthInBytes(Ty) == 1;
const bool IsByte = isByteSizedType(Ty);
// Check conditions of rules violated.
if (getGPRReg(kRnShift, Address) == RegARM32::Encoded_Reg_pc)
return setNeedsTextFixup();
......@@ -873,7 +874,7 @@ void AssemblerARM32::str(const Operand *OpRt, const Operand *OpAddress,
const Type Ty = OpRt->getType();
if (!(Ty == IceType_i32 || Ty == IceType_i8)) // TODO(kschimpf) Expand?
return setNeedsTextFixup();
const bool IsByte = typeWidthInBytes(Ty) == 1;
const bool IsByte = isByteSizedType(Ty);
// Check for rule violations.
if ((getGPRReg(kRnShift, Address) == RegARM32::Encoded_Reg_pc))
return setNeedsTextFixup();
......
src/IceInstARM32.cpp
......@@ -700,15 +700,14 @@ void InstARM32Mov::emitSingleDestSingleSource(const Cfg *Func) const {
Variable *Dest = getDest();
if (Dest->hasReg()) {
Type DestTy = Dest->getType();
Type Ty = Dest->getType();
Operand *Src0 = getSrc(0);
const bool DestIsVector = isVectorType(DestTy);
const bool DestIsScalarFP = isScalarFloatingType(Dest->getType());
const bool IsVector = isVectorType(Ty);
const bool IsScalarFP = isScalarFloatingType(Ty);
const bool CoreVFPMove = isMoveBetweenCoreAndVFPRegisters(Dest, Src0);
const char *LoadOpcode =
DestIsVector ? "vld1" : (DestIsScalarFP ? "vldr" : "ldr");
const char *RegMovOpcode =
(DestIsVector || DestIsScalarFP || CoreVFPMove) ? "vmov" : "mov";
const char *LoadOpcode = IsVector ? "vld1" : (IsScalarFP ? "vldr" : "ldr");
const bool IsVMove = (IsVector || IsScalarFP || CoreVFPMove);
const char *RegMovOpcode = IsVMove ? "vmov" : "mov";
const char *ActualOpcode = isMemoryAccess(Src0) ? LoadOpcode : RegMovOpcode;
// when vmov{c}'ing, we need to emit a width string. Otherwise, the
// assembler might be tempted to assume we want a vector vmov{c}, and that
......@@ -716,24 +715,36 @@ void InstARM32Mov::emitSingleDestSingleSource(const Cfg *Func) const {
const char *NoWidthString = "";
const char *WidthString =
isMemoryAccess(Src0)
? (DestIsVector ? ".64" : NoWidthString)
: (!CoreVFPMove ? getVecWidthString(DestTy) : NoWidthString);
Str << "\t" << ActualOpcode << getPredicate() << WidthString << "\t";
? (IsVector ? ".64" : getWidthString(Ty))
: (!CoreVFPMove ? getVecWidthString(Ty) : NoWidthString);
Str << "\t" << ActualOpcode;
const bool IsVInst = IsVMove || IsVector || IsScalarFP;
if (IsVInst) {
Str << getPredicate() << WidthString;
} else {
Str << WidthString << getPredicate();
}
Str << "\t";
Dest->emit(Func);
Str << ", ";
Src0->emit(Func);
} else {
Variable *Src0 = llvm::cast<Variable>(getSrc(0));
assert(Src0->hasReg());
Type Ty = Src0->getType();
const bool IsVector = isVectorType(Ty);
const bool IsScalarFP = isScalarFloatingType(Ty);
const char *ActualOpcode =
isVectorType(Src0->getType())
? "vst1"
: (isScalarFloatingType(Src0->getType()) ? "vstr" : "str");
const char *NoWidthString = "";
const char *WidthString =
isVectorType(Src0->getType()) ? ".64" : NoWidthString;
Str << "\t" << ActualOpcode << getPredicate() << WidthString << "\t";
IsVector ? "vst1" : (IsScalarFP ? "vstr" : "str");
const char *WidthString = IsVector ? ".64" : getWidthString(Ty);
Str << "\t" << ActualOpcode;
const bool IsVInst = IsVector || IsScalarFP;
if (IsVInst) {
Str << getPredicate() << WidthString;
} else {
Str << WidthString << getPredicate();
}
Str << "\t";
Src0->emit(Func);
Str << ", ";
Dest->emit(Func);
......@@ -955,15 +966,21 @@ template <> void InstARM32Ldr::emit(const Cfg *Func) const {
assert(getSrcSize() == 1);
assert(getDest()->hasReg());
Variable *Dest = getDest();
Type DestTy = Dest->getType();
const bool DestIsVector = isVectorType(DestTy);
const bool DestIsScalarFloat = isScalarFloatingType(DestTy);
Type Ty = Dest->getType();
const bool IsVector = isVectorType(Ty);
const bool IsScalarFloat = isScalarFloatingType(Ty);
const char *ActualOpcode =
DestIsVector ? "vld1" : (DestIsScalarFloat ? "vldr" : "ldr");
const char *VectorMarker = DestIsVector ? ".64" : "";
const char *WidthString = DestIsVector ? "" : getWidthString(DestTy);
Str << "\t" << ActualOpcode << WidthString << getPredicate() << VectorMarker
<< "\t";
IsVector ? "vld1" : (IsScalarFloat ? "vldr" : "ldr");
const char *VectorMarker = IsVector ? ".64" : "";
const char *WidthString = IsVector ? "" : getWidthString(Ty);
Str << "\t" << ActualOpcode;
const bool IsVInst = IsVector || IsScalarFloat;
if (IsVInst) {
Str << getPredicate() << WidthString;
} else {
Str << WidthString << getPredicate();
}
Str << VectorMarker << "\t";
getDest()->emit(Func);
Str << ", ";
getSrc(0)->emit(Func);
......@@ -1270,11 +1287,18 @@ void InstARM32Str::emit(const Cfg *Func) const {
assert(getSrcSize() == 2);
Type Ty = getSrc(0)->getType();
const bool IsVectorStore = isVectorType(Ty);
const bool IsScalarFloat = isScalarFloatingType(Ty);
const char *Opcode =
IsVectorStore ? "vst1" : (isScalarFloatingType(Ty) ? "vstr" : "str");
IsVectorStore ? "vst1" : (IsScalarFloat ? "vstr" : "str");
const char *VecEltWidthString = IsVectorStore ? ".64" : "";
Str << "\t" << Opcode << getWidthString(Ty) << getPredicate()
<< VecEltWidthString << "\t";
Str << "\t" << Opcode;
const bool IsVInst = IsVectorStore || IsScalarFloat;
if (IsVInst) {
Str << getPredicate() << getWidthString(Ty);
} else {
Str << getWidthString(Ty) << getPredicate();
}
Str << VecEltWidthString << "\t";
getSrc(0)->emit(Func);
Str << ", ";
getSrc(1)->emit(Func);
......
src/IceTargetLoweringARM32.cpp
......@@ -2506,7 +2506,7 @@ void TargetARM32::lowerFcmp(const InstFcmp *Instr) {
return;
}
Variable *T = makeReg(IceType_i32);
Variable *T = makeReg(IceType_i1);
Operand *_1 = Ctx->getConstantInt32(1);
Operand *_0 = Ctx->getConstantZero(IceType_i32);
......@@ -2672,7 +2672,7 @@ void TargetARM32::lowerIcmp(const InstIcmp *Inst) {
Constant *_0 = Ctx->getConstantZero(IceType_i32);
Constant *_1 = Ctx->getConstantInt32(1);
Variable *T = makeReg(IceType_i32);
Variable *T = makeReg(IceType_i1);
CondARM32::Cond CondIfTrue, CondIfFalse;
lowerIcmpCond(Inst, &CondIfTrue, &CondIfFalse);
......
tests_lit/assembler/arm32/branch-mult-fwd.ll
......@@ -57,14 +57,14 @@ define internal void @mult_fwd_branches(i32 %a, i32 %b) {
; ASM-NEXT: cmp r0, r1
; ASM-NEXT: movge r0, #0
; ASM-NEXT: movlt r0, #1
; ASM-NEXT: str r0, [sp]
; ASM-NEXT: strb r0, [sp]
; DIS-NEXT: c: e59d0008
; DIS-NEXT: 10: e59d1004
; DIS-NEXT: 14: e1500001
; DIS-NEXT: 18: a3a00000
; DIS-NEXT: 1c: b3a00001
; DIS-NEXT: 20: e58d0000
; DIS-NEXT: 20: e5cd0000
; IASM-NEXT: .byte 0x8
; IASM-NEXT: .byte 0x0
......@@ -91,26 +91,23 @@ define internal void @mult_fwd_branches(i32 %a, i32 %b) {
; IASM-NEXT: .byte 0xa0
; IASM-NEXT: .byte 0xb3
; IASM-NEXT: .byte 0x0
; IASM-NEXT: .byte 0x0
; IASM-NEXT: .byte 0x8d
; IASM-NEXT: .byte 0xe5
; IASM-NEXT: strb r0, [sp]
br i1 %cmp, label %then, label %else
; ASM-NEXT: ldr r0, [sp]
; ASM-NEXT: ldrb r0, [sp]
; ASM-NEXT: uxtb r0, r0
; ASM-NEXT: cmp r0, #0
; ASM-NEXT: bne .Lmult_fwd_branches$then
; ASM-NEXT: b .Lmult_fwd_branches$else
; DIS-NEXT: 24: e59d0000
; DIS-NEXT: 24: e5dd0000
; DIS-NEXT: 28: e6ef0070
; DIS-NEXT: 2c: e3500000
; DIS-NEXT: 30: 1a000000
; DIS-NEXT: 34: ea000000
; IASM-NEXT: ldr r0, [sp]
; IASM-NEXT: ldrb r0, [sp]
; IASM-NEXT: uxtb r0, r0
; IASM-NEXT: .byte 0x0
......
tests_lit/llvm2ice_tests/nacl-atomic-intrinsics.ll
......@@ -1068,7 +1068,7 @@ entry:
; ARM32: ldrexb
; ARM32: cmp
; ARM32: strexbeq
; ARM32: {{str|mov}}ne
; ARM32: {{strb|mov}}ne
; ARM32: cmpeq
; ARM32: bne
; ARM32: dmb
......@@ -1092,7 +1092,7 @@ entry:
; ARM32: ldrexh
; ARM32: cmp
; ARM32: strexheq
; ARM32: {{str|mov}}ne
; ARM32: {{strh|mov}}ne
; ARM32: cmpeq
; ARM32: bne
; ARM32: dmb
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment