[arm] Don't overwrite input in atomic ops

The Arm atomic operations used an input register as a temporary,
corrupting it for future uses. Fix this by adding another temporary,
the impact of which is partly reduced by removing the "unique"
requirement on the address base register.

Bug: 
Change-Id: I99a7bc3c14100d64cb7478e2053cf83ab6dcea50
Reviewed-on: https://chromium-review.googlesource.com/795953
Commit-Queue: Martyn Capewell <martyn.capewell@arm.com>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49785}
This commit is contained in:
Martyn Capewell 2017-12-01 11:00:37 +00:00 committed by Commit Bot
parent 968a2f66a9
commit c010a3819d
2 changed files with 70 additions and 63 deletions

View File

@ -432,51 +432,51 @@ Condition FlagsConditionToCondition(FlagsCondition condition) {
__ dmb(ISH); \ __ dmb(ISH); \
} while (0) } while (0)
#define ASSEMBLE_ATOMIC_EXCHANGE_INTEGER(load_instr, store_instr) \ #define ASSEMBLE_ATOMIC_EXCHANGE_INTEGER(load_instr, store_instr) \
do { \
Label exchange; \
__ add(i.InputRegister(0), i.InputRegister(0), i.InputRegister(1)); \
__ dmb(ISH); \
__ bind(&exchange); \
__ load_instr(i.OutputRegister(0), i.InputRegister(0)); \
__ store_instr(i.TempRegister(0), i.InputRegister(2), i.InputRegister(0)); \
__ teq(i.TempRegister(0), Operand(0)); \
__ b(ne, &exchange); \
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(load_instr, store_instr) \
do { \
Label compareExchange; \
Label exit; \
__ add(i.InputRegister(0), i.InputRegister(0), i.InputRegister(1)); \
__ dmb(ISH); \
__ bind(&compareExchange); \
__ load_instr(i.OutputRegister(0), i.InputRegister(0)); \
__ teq(i.InputRegister(2), Operand(i.OutputRegister(0))); \
__ b(ne, &exit); \
__ store_instr(i.TempRegister(0), i.InputRegister(3), i.InputRegister(0)); \
__ teq(i.TempRegister(0), Operand(0)); \
__ b(ne, &compareExchange); \
__ bind(&exit); \
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_ATOMIC_BINOP(load_instr, store_instr, bin_instr) \
do { \ do { \
Label binop; \ Label exchange; \
__ add(i.InputRegister(0), i.InputRegister(0), i.InputRegister(1)); \ __ add(i.TempRegister(1), i.InputRegister(0), i.InputRegister(1)); \
__ dmb(ISH); \ __ dmb(ISH); \
__ bind(&binop); \ __ bind(&exchange); \
__ load_instr(i.OutputRegister(0), i.InputRegister(0)); \ __ load_instr(i.OutputRegister(0), i.TempRegister(1)); \
__ bin_instr(i.TempRegister(0), i.OutputRegister(0), \ __ store_instr(i.TempRegister(0), i.InputRegister(2), i.TempRegister(1)); \
Operand(i.InputRegister(2))); \ __ teq(i.TempRegister(0), Operand(0)); \
__ store_instr(i.TempRegister(1), i.TempRegister(0), i.InputRegister(0)); \ __ b(ne, &exchange); \
__ teq(i.TempRegister(1), Operand(0)); \
__ b(ne, &binop); \
__ dmb(ISH); \ __ dmb(ISH); \
} while (0) } while (0)
#define ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(load_instr, store_instr, \
cmp_reg) \
do { \
Label compareExchange; \
Label exit; \
__ dmb(ISH); \
__ bind(&compareExchange); \
__ load_instr(i.OutputRegister(0), i.TempRegister(1)); \
__ teq(cmp_reg, Operand(i.OutputRegister(0))); \
__ b(ne, &exit); \
__ store_instr(i.TempRegister(0), i.InputRegister(3), i.TempRegister(1)); \
__ teq(i.TempRegister(0), Operand(0)); \
__ b(ne, &compareExchange); \
__ bind(&exit); \
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_ATOMIC_BINOP(load_instr, store_instr, bin_instr) \
do { \
Label binop; \
__ add(i.TempRegister(1), i.InputRegister(0), i.InputRegister(1)); \
__ dmb(ISH); \
__ bind(&binop); \
__ load_instr(i.OutputRegister(0), i.TempRegister(1)); \
__ bin_instr(i.TempRegister(0), i.OutputRegister(0), \
Operand(i.InputRegister(2))); \
__ store_instr(i.TempRegister(2), i.TempRegister(0), i.TempRegister(1)); \
__ teq(i.TempRegister(2), Operand(0)); \
__ b(ne, &binop); \
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_IEEE754_BINOP(name) \ #define ASSEMBLE_IEEE754_BINOP(name) \
do { \ do { \
/* TODO(bmeurer): We should really get rid of this special instruction, */ \ /* TODO(bmeurer): We should really get rid of this special instruction, */ \
@ -2642,25 +2642,35 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
ASSEMBLE_ATOMIC_EXCHANGE_INTEGER(ldrex, strex); ASSEMBLE_ATOMIC_EXCHANGE_INTEGER(ldrex, strex);
break; break;
case kAtomicCompareExchangeInt8: case kAtomicCompareExchangeInt8:
__ uxtb(i.InputRegister(2), i.InputRegister(2)); __ add(i.TempRegister(1), i.InputRegister(0), i.InputRegister(1));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexb, strexb); __ uxtb(i.TempRegister(2), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexb, strexb,
i.TempRegister(2));
__ sxtb(i.OutputRegister(0), i.OutputRegister(0)); __ sxtb(i.OutputRegister(0), i.OutputRegister(0));
break; break;
case kAtomicCompareExchangeUint8: case kAtomicCompareExchangeUint8:
__ uxtb(i.InputRegister(2), i.InputRegister(2)); __ add(i.TempRegister(1), i.InputRegister(0), i.InputRegister(1));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexb, strexb); __ uxtb(i.TempRegister(2), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexb, strexb,
i.TempRegister(2));
break; break;
case kAtomicCompareExchangeInt16: case kAtomicCompareExchangeInt16:
__ uxth(i.InputRegister(2), i.InputRegister(2)); __ add(i.TempRegister(1), i.InputRegister(0), i.InputRegister(1));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexh, strexh); __ uxth(i.TempRegister(2), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexh, strexh,
i.TempRegister(2));
__ sxth(i.OutputRegister(0), i.OutputRegister(0)); __ sxth(i.OutputRegister(0), i.OutputRegister(0));
break; break;
case kAtomicCompareExchangeUint16: case kAtomicCompareExchangeUint16:
__ uxth(i.InputRegister(2), i.InputRegister(2)); __ add(i.TempRegister(1), i.InputRegister(0), i.InputRegister(1));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexh, strexh); __ uxth(i.TempRegister(2), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexh, strexh,
i.TempRegister(2));
break; break;
case kAtomicCompareExchangeWord32: case kAtomicCompareExchangeWord32:
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrex, strex); __ add(i.TempRegister(1), i.InputRegister(0), i.InputRegister(1));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrex, strex,
i.InputRegister(2));
break; break;
#define ATOMIC_BINOP_CASE(op, inst) \ #define ATOMIC_BINOP_CASE(op, inst) \
case kAtomic##op##Int8: \ case kAtomic##op##Int8: \

View File

@ -2276,15 +2276,14 @@ void InstructionSelector::VisitAtomicExchange(Node* node) {
AddressingMode addressing_mode = kMode_Offset_RR; AddressingMode addressing_mode = kMode_Offset_RR;
InstructionOperand inputs[3]; InstructionOperand inputs[3];
size_t input_count = 0; size_t input_count = 0;
inputs[input_count++] = g.UseUniqueRegister(base); inputs[input_count++] = g.UseRegister(base);
inputs[input_count++] = g.UseRegister(index); inputs[input_count++] = g.UseRegister(index);
inputs[input_count++] = g.UseUniqueRegister(value); inputs[input_count++] = g.UseUniqueRegister(value);
InstructionOperand outputs[1]; InstructionOperand outputs[1];
outputs[0] = g.DefineAsRegister(node); outputs[0] = g.DefineAsRegister(node);
InstructionOperand temp[1]; InstructionOperand temps[] = {g.TempRegister(), g.TempRegister()};
temp[0] = g.TempRegister();
InstructionCode code = opcode | AddressingModeField::encode(addressing_mode); InstructionCode code = opcode | AddressingModeField::encode(addressing_mode);
Emit(code, 1, outputs, input_count, inputs, 1, temp); Emit(code, 1, outputs, input_count, inputs, arraysize(temps), temps);
} }
void InstructionSelector::VisitAtomicCompareExchange(Node* node) { void InstructionSelector::VisitAtomicCompareExchange(Node* node) {
@ -2313,16 +2312,16 @@ void InstructionSelector::VisitAtomicCompareExchange(Node* node) {
AddressingMode addressing_mode = kMode_Offset_RR; AddressingMode addressing_mode = kMode_Offset_RR;
InstructionOperand inputs[4]; InstructionOperand inputs[4];
size_t input_count = 0; size_t input_count = 0;
inputs[input_count++] = g.UseUniqueRegister(base); inputs[input_count++] = g.UseRegister(base);
inputs[input_count++] = g.UseRegister(index); inputs[input_count++] = g.UseRegister(index);
inputs[input_count++] = g.UseUniqueRegister(old_value); inputs[input_count++] = g.UseUniqueRegister(old_value);
inputs[input_count++] = g.UseUniqueRegister(new_value); inputs[input_count++] = g.UseUniqueRegister(new_value);
InstructionOperand outputs[1]; InstructionOperand outputs[1];
outputs[0] = g.DefineAsRegister(node); outputs[0] = g.DefineAsRegister(node);
InstructionOperand temp[1]; InstructionOperand temps[] = {g.TempRegister(), g.TempRegister(),
temp[0] = g.TempRegister(); g.TempRegister()};
InstructionCode code = opcode | AddressingModeField::encode(addressing_mode); InstructionCode code = opcode | AddressingModeField::encode(addressing_mode);
Emit(code, 1, outputs, input_count, inputs, 1, temp); Emit(code, 1, outputs, input_count, inputs, arraysize(temps), temps);
} }
void InstructionSelector::VisitAtomicBinaryOperation( void InstructionSelector::VisitAtomicBinaryOperation(
@ -2352,17 +2351,15 @@ void InstructionSelector::VisitAtomicBinaryOperation(
AddressingMode addressing_mode = kMode_Offset_RR; AddressingMode addressing_mode = kMode_Offset_RR;
InstructionOperand inputs[3]; InstructionOperand inputs[3];
size_t input_count = 0; size_t input_count = 0;
inputs[input_count++] = g.UseUniqueRegister(base); inputs[input_count++] = g.UseRegister(base);
inputs[input_count++] = g.UseRegister(index); inputs[input_count++] = g.UseRegister(index);
inputs[input_count++] = g.UseUniqueRegister(value); inputs[input_count++] = g.UseUniqueRegister(value);
InstructionOperand outputs[1]; InstructionOperand outputs[1];
outputs[0] = g.DefineAsRegister(node); outputs[0] = g.DefineAsRegister(node);
InstructionOperand temps[2]; InstructionOperand temps[] = {g.TempRegister(), g.TempRegister(),
size_t temp_count = 0; g.TempRegister()};
temps[temp_count++] = g.TempRegister();
temps[temp_count++] = g.TempRegister();
InstructionCode code = opcode | AddressingModeField::encode(addressing_mode); InstructionCode code = opcode | AddressingModeField::encode(addressing_mode);
Emit(code, 1, outputs, input_count, inputs, temp_count, temps); Emit(code, 1, outputs, input_count, inputs, arraysize(temps), temps);
} }
#define VISIT_ATOMIC_BINOP(op) \ #define VISIT_ATOMIC_BINOP(op) \