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 .
diff --git a/src/IceAssemblerARM32.cpp b/src/IceAssemblerARM32.cpp index 338022f..1dab8b8 100644 --- a/src/IceAssemblerARM32.cpp +++ b/src/IceAssemblerARM32.cpp
@@ -705,6 +705,7 @@ // 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 @@ 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 @@ 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();
diff --git a/src/IceInstARM32.cpp b/src/IceInstARM32.cpp index 9e6a480..376a2cf 100644 --- a/src/IceInstARM32.cpp +++ b/src/IceInstARM32.cpp
@@ -700,15 +700,14 @@ 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 @@ 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 @@ 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 @@ 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);
diff --git a/src/IceTargetLoweringARM32.cpp b/src/IceTargetLoweringARM32.cpp index 31ef0c0..d2aa74e 100644 --- a/src/IceTargetLoweringARM32.cpp +++ b/src/IceTargetLoweringARM32.cpp
@@ -2506,7 +2506,7 @@ 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 @@ 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);
diff --git a/tests_lit/assembler/arm32/branch-mult-fwd.ll b/tests_lit/assembler/arm32/branch-mult-fwd.ll index b0cf629..89c495d 100644 --- a/tests_lit/assembler/arm32/branch-mult-fwd.ll +++ b/tests_lit/assembler/arm32/branch-mult-fwd.ll
@@ -57,14 +57,14 @@ ; 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 @@ ; 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
diff --git a/tests_lit/llvm2ice_tests/nacl-atomic-intrinsics.ll b/tests_lit/llvm2ice_tests/nacl-atomic-intrinsics.ll index d3c228c..0415c25 100644 --- a/tests_lit/llvm2ice_tests/nacl-atomic-intrinsics.ll +++ b/tests_lit/llvm2ice_tests/nacl-atomic-intrinsics.ll
@@ -1068,7 +1068,7 @@ ; ARM32: ldrexb ; ARM32: cmp ; ARM32: strexbeq -; ARM32: {{str|mov}}ne +; ARM32: {{strb|mov}}ne ; ARM32: cmpeq ; ARM32: bne ; ARM32: dmb @@ -1092,7 +1092,7 @@ ; ARM32: ldrexh ; ARM32: cmp ; ARM32: strexheq -; ARM32: {{str|mov}}ne +; ARM32: {{strh|mov}}ne ; ARM32: cmpeq ; ARM32: bne ; ARM32: dmb