Skip to content

Commit 550d2ea

Browse files
PenguinwizzardPenguinwizzard
committed
[MERGE #5217 @Penguinwizzard] Fix ARM64 LEA legalization
Merge pull request #5217 from Penguinwizzard:fix_arm64_operandlength Because we weren't checking the types of things, there were a few ways that the conversion of LEA instructions to ADD instructions on ARM64 could go wrong. This changeset fixes legalization of these instructions to properly handle all of the cases.
2 parents 8544540 + de04a97 commit 550d2ea

File tree

7 files changed

+179
-131
lines changed

7 files changed

+179
-131
lines changed

‎lib/Backend/IR.cpp

Lines changed: 0 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -2521,54 +2521,6 @@ Instr::HoistSrc2(Js::OpCode assignOpcode, RegNum regNum, StackSym *newSym)
25212521
return newInstr;
25222522
}
25232523

2524-
///----------------------------------------------------------------------------
2525-
///
2526-
/// Instr::HoistIndirOffset
2527-
///
2528-
/// Replace the offset of the given indir with a new symbol, which becomes the indir index.
2529-
/// Assign the new symbol by creating an assignment from the constant offset.
2530-
///
2531-
///----------------------------------------------------------------------------
2532-
2533-
Instr *
2534-
Instr::HoistIndirOffset(IR::IndirOpnd *indirOpnd, RegNum regNum)
2535-
{
2536-
int32 offset = indirOpnd->GetOffset();
2537-
if (indirOpnd->GetIndexOpnd())
2538-
{
2539-
Assert(indirOpnd->GetBaseOpnd());
2540-
return HoistIndirOffsetAsAdd(indirOpnd, indirOpnd->GetBaseOpnd(), offset, regNum);
2541-
}
2542-
IntConstOpnd *offsetOpnd = IntConstOpnd::New(offset, TyInt32, this->m_func);
2543-
RegOpnd *indexOpnd = RegOpnd::New(StackSym::New(TyMachReg, this->m_func), regNum, TyMachReg, this->m_func);
2544-
2545-
#if defined(DBG) && defined(_M_ARM)
2546-
if (regNum == SCRATCH_REG)
2547-
{
2548-
AssertMsg(indirOpnd->GetBaseOpnd()->GetReg()!= SCRATCH_REG, "Why both are SCRATCH_REG");
2549-
if (this->GetSrc1() && this->GetSrc1()->IsRegOpnd())
2550-
{
2551-
Assert(this->GetSrc1()->AsRegOpnd()->GetReg() != SCRATCH_REG);
2552-
}
2553-
if (this->GetSrc2() && this->GetSrc2()->IsRegOpnd())
2554-
{
2555-
Assert(this->GetSrc2()->AsRegOpnd()->GetReg() != SCRATCH_REG);
2556-
}
2557-
if (this->GetDst() && this->GetDst()->IsRegOpnd())
2558-
{
2559-
Assert(this->GetDst()->AsRegOpnd()->GetReg() != SCRATCH_REG);
2560-
}
2561-
}
2562-
#endif
2563-
// Clear the offset and add a new reg as the index.
2564-
indirOpnd->SetOffset(0);
2565-
indirOpnd->SetIndexOpnd(indexOpnd);
2566-
2567-
Instr *instrAssign = Lowerer::InsertMove(indexOpnd, offsetOpnd, this);
2568-
indexOpnd->m_sym->SetIsIntConst(offset);
2569-
return instrAssign;
2570-
}
2571-
25722524
IndirOpnd *
25732525
Instr::HoistMemRefAddress(MemRefOpnd *const memRefOpnd, const Js::OpCode loadOpCode)
25742526
{
@@ -2641,71 +2593,6 @@ Opnd *Instr::DeepReplace(Opnd *const oldOpnd, Opnd *const newOpnd)
26412593
return Replace(oldOpnd, newOpnd);
26422594
}
26432595

2644-
Instr *
2645-
Instr::HoistIndirOffsetAsAdd(IR::IndirOpnd *orgOpnd, IR::Opnd *baseOpnd, int offset, RegNum regNum)
2646-
{
2647-
IR::RegOpnd *newBaseOpnd = IR::RegOpnd::New(StackSym::New(TyMachPtr, this->m_func), regNum, TyMachPtr, this->m_func);
2648-
2649-
IR::IntConstOpnd *src2 = IR::IntConstOpnd::New(offset, TyInt32, this->m_func);
2650-
IR::Instr * instrAdd = IR::Instr::New(Js::OpCode::ADD, newBaseOpnd, baseOpnd, src2, this->m_func);
2651-
2652-
this->InsertBefore(instrAdd);
2653-
2654-
orgOpnd->ReplaceBaseOpnd(newBaseOpnd);
2655-
orgOpnd->SetOffset(0);
2656-
2657-
return instrAdd;
2658-
}
2659-
2660-
Instr *
2661-
Instr::HoistIndirIndexOpndAsAdd(IR::IndirOpnd *orgOpnd, IR::Opnd *baseOpnd, IR::Opnd *indexOpnd, RegNum regNum)
2662-
{
2663-
IR::RegOpnd *newBaseOpnd = IR::RegOpnd::New(StackSym::New(TyMachPtr, this->m_func), regNum, TyMachPtr, this->m_func);
2664-
2665-
IR::Instr * instrAdd = IR::Instr::New(Js::OpCode::ADD, newBaseOpnd, baseOpnd, indexOpnd->UseWithNewType(TyMachPtr, this->m_func), this->m_func);
2666-
2667-
this->InsertBefore(instrAdd);
2668-
2669-
orgOpnd->ReplaceBaseOpnd(newBaseOpnd);
2670-
orgOpnd->SetIndexOpnd(nullptr);
2671-
2672-
return instrAdd;
2673-
}
2674-
2675-
Instr *
2676-
Instr::HoistSymOffsetAsAdd(IR::SymOpnd *orgOpnd, IR::Opnd *baseOpnd, int offset, RegNum regNum)
2677-
{
2678-
IR::IndirOpnd *newIndirOpnd = IR::IndirOpnd::New(baseOpnd->AsRegOpnd(), 0, TyMachPtr, this->m_func);
2679-
this->Replace(orgOpnd, newIndirOpnd); // Replace SymOpnd with IndirOpnd
2680-
return this->HoistIndirOffsetAsAdd(newIndirOpnd, baseOpnd, offset, regNum);
2681-
}
2682-
2683-
///----------------------------------------------------------------------------
2684-
///
2685-
/// Instr::HoistSymOffset
2686-
///
2687-
/// Replace the given sym with an indir using the given base and offset.
2688-
/// (This is used, for instance, to hoist a sym offset that is too large to encode.)
2689-
///
2690-
///----------------------------------------------------------------------------
2691-
2692-
Instr *
2693-
Instr::HoistSymOffset(SymOpnd *symOpnd, RegNum baseReg, uint32 offset, RegNum regNum)
2694-
{
2695-
IR::RegOpnd *baseOpnd = IR::RegOpnd::New(nullptr, baseReg, TyMachPtr, this->m_func);
2696-
IR::IndirOpnd *indirOpnd = IR::IndirOpnd::New(baseOpnd, offset, symOpnd->GetType(), this->m_func);
2697-
if (symOpnd == this->GetDst())
2698-
{
2699-
this->ReplaceDst(indirOpnd);
2700-
}
2701-
else
2702-
{
2703-
this->ReplaceSrc(symOpnd, indirOpnd);
2704-
}
2705-
2706-
return this->HoistIndirOffset(indirOpnd, regNum);
2707-
}
2708-
27092596
Opnd *
27102597
Instr::UnlinkSrc(Opnd *src)
27112598
{

‎lib/Backend/IR.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -265,11 +265,6 @@ class Instr
265265
void FreeSrc2();
266266
Opnd * ReplaceSrc2(Opnd * newSrc);
267267
Instr * HoistSrc2(Js::OpCode assignOpcode, RegNum regNum = RegNOREG, StackSym *newSym = nullptr);
268-
Instr * HoistIndirOffset(IndirOpnd *indirOpnd, RegNum regNum = RegNOREG);
269-
Instr * HoistSymOffset(SymOpnd *symOpnd, RegNum baseReg, uint32 offset, RegNum regNum = RegNOREG);
270-
Instr * HoistIndirOffsetAsAdd(IndirOpnd *orgOpnd, IR::Opnd *baseOpnd, int offset, RegNum regNum);
271-
Instr * HoistSymOffsetAsAdd(SymOpnd *orgOpnd, IR::Opnd *baseOpnd, int offset, RegNum regNum);
272-
Instr * HoistIndirIndexOpndAsAdd(IR::IndirOpnd *orgOpnd, IR::Opnd *baseOpnd, IR::Opnd *indexOpnd, RegNum regNum);
273268
IndirOpnd * HoistMemRefAddress(MemRefOpnd *const memRefOpnd, const Js::OpCode loadOpCode);
274269
Opnd * UnlinkSrc(Opnd *src);
275270
Opnd * ReplaceSrc(Opnd *oldSrc, Opnd * newSrc);

‎lib/Backend/Lower.cpp

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14881,6 +14881,110 @@ IR::RegOpnd *Lowerer::GenerateArrayTest(
1488114881
return arrayOpnd;
1488214882
}
1488314883

14884+
///----------------------------------------------------------------------------
14885+
///
14886+
/// Instr::HoistIndirOffset
14887+
///
14888+
/// Replace the offset of the given indir with a new symbol, which becomes the indir index.
14889+
/// Assign the new symbol by creating an assignment from the constant offset.
14890+
///
14891+
///----------------------------------------------------------------------------
14892+
14893+
IR::Instr *Lowerer::HoistIndirOffset(IR::Instr* instr, IR::IndirOpnd *indirOpnd, RegNum regNum)
14894+
{
14895+
int32 offset = indirOpnd->GetOffset();
14896+
if (indirOpnd->GetIndexOpnd())
14897+
{
14898+
Assert(indirOpnd->GetBaseOpnd());
14899+
return Lowerer::HoistIndirOffsetAsAdd(instr, indirOpnd, indirOpnd->GetBaseOpnd(), offset, regNum);
14900+
}
14901+
IR::IntConstOpnd *offsetOpnd = IR::IntConstOpnd::New(offset, TyInt32, instr->m_func);
14902+
IR::RegOpnd *indexOpnd = IR::RegOpnd::New(StackSym::New(TyMachReg, instr->m_func), regNum, TyMachReg, instr->m_func);
14903+
14904+
#if defined(DBG) && defined(_M_ARM)
14905+
if (regNum == SCRATCH_REG)
14906+
{
14907+
AssertMsg(indirOpnd->GetBaseOpnd()->GetReg()!= SCRATCH_REG, "Why both are SCRATCH_REG");
14908+
if (instr->GetSrc1() && instr->GetSrc1()->IsRegOpnd())
14909+
{
14910+
Assert(instr->GetSrc1()->AsRegOpnd()->GetReg() != SCRATCH_REG);
14911+
}
14912+
if (instr->GetSrc2() && instr->GetSrc2()->IsRegOpnd())
14913+
{
14914+
Assert(instr->GetSrc2()->AsRegOpnd()->GetReg() != SCRATCH_REG);
14915+
}
14916+
if (instr->GetDst() && instr->GetDst()->IsRegOpnd())
14917+
{
14918+
Assert(instr->GetDst()->AsRegOpnd()->GetReg() != SCRATCH_REG);
14919+
}
14920+
}
14921+
#endif
14922+
// Clear the offset and add a new reg as the index.
14923+
indirOpnd->SetOffset(0);
14924+
indirOpnd->SetIndexOpnd(indexOpnd);
14925+
14926+
IR::Instr *instrAssign = Lowerer::InsertMove(indexOpnd, offsetOpnd, instr);
14927+
indexOpnd->m_sym->SetIsIntConst(offset);
14928+
return instrAssign;
14929+
}
14930+
14931+
IR::Instr *Lowerer::HoistIndirOffsetAsAdd(IR::Instr* instr, IR::IndirOpnd *orgOpnd, IR::Opnd *baseOpnd, int offset, RegNum regNum)
14932+
{
14933+
IR::RegOpnd *newBaseOpnd = IR::RegOpnd::New(StackSym::New(TyMachPtr, instr->m_func), regNum, TyMachPtr, instr->m_func);
14934+
14935+
IR::IntConstOpnd *src2 = IR::IntConstOpnd::New(offset, TyInt32, instr->m_func);
14936+
IR::Instr * instrAdd = Lowerer::InsertAdd(false, newBaseOpnd, baseOpnd, src2, instr);
14937+
14938+
orgOpnd->ReplaceBaseOpnd(newBaseOpnd);
14939+
orgOpnd->SetOffset(0);
14940+
14941+
return instrAdd;
14942+
}
14943+
14944+
IR::Instr *Lowerer::HoistIndirIndexOpndAsAdd(IR::Instr* instr, IR::IndirOpnd *orgOpnd, IR::Opnd *baseOpnd, IR::Opnd *indexOpnd, RegNum regNum)
14945+
{
14946+
IR::RegOpnd *newBaseOpnd = IR::RegOpnd::New(StackSym::New(TyMachPtr, instr->m_func), regNum, TyMachPtr, instr->m_func);
14947+
14948+
IR::Instr * instrAdd = Lowerer::InsertAdd(false, newBaseOpnd, baseOpnd, indexOpnd->UseWithNewType(TyMachPtr, instr->m_func), instr);
14949+
14950+
orgOpnd->ReplaceBaseOpnd(newBaseOpnd);
14951+
orgOpnd->SetIndexOpnd(nullptr);
14952+
14953+
return instrAdd;
14954+
}
14955+
14956+
///----------------------------------------------------------------------------
14957+
///
14958+
/// Instr::HoistSymOffset
14959+
///
14960+
/// Replace the given sym with an indir using the given base and offset.
14961+
/// (This is used, for instance, to hoist a sym offset that is too large to encode.)
14962+
///
14963+
///----------------------------------------------------------------------------
14964+
14965+
IR::Instr *Lowerer::HoistSymOffset(IR::Instr *instr, IR::SymOpnd *symOpnd, RegNum baseReg, uint32 offset, RegNum regNum)
14966+
{
14967+
IR::RegOpnd *baseOpnd = IR::RegOpnd::New(nullptr, baseReg, TyMachPtr, instr->m_func);
14968+
IR::IndirOpnd *indirOpnd = IR::IndirOpnd::New(baseOpnd, offset, symOpnd->GetType(), instr->m_func);
14969+
if (symOpnd == instr->GetDst())
14970+
{
14971+
instr->ReplaceDst(indirOpnd);
14972+
}
14973+
else
14974+
{
14975+
instr->ReplaceSrc(symOpnd, indirOpnd);
14976+
}
14977+
14978+
return Lowerer::HoistIndirOffset(instr, indirOpnd, regNum);
14979+
}
14980+
14981+
IR::Instr *Lowerer::HoistSymOffsetAsAdd(IR::Instr* instr, IR::SymOpnd *orgOpnd, IR::Opnd *baseOpnd, int offset, RegNum regNum)
14982+
{
14983+
IR::IndirOpnd *newIndirOpnd = IR::IndirOpnd::New(baseOpnd->AsRegOpnd(), 0, TyMachPtr, instr->m_func);
14984+
instr->Replace(orgOpnd, newIndirOpnd); // Replace SymOpnd with IndirOpnd
14985+
return Lowerer::HoistIndirOffsetAsAdd(instr, newIndirOpnd, baseOpnd, offset, regNum);
14986+
}
14987+
1488414988
IR::LabelInstr *Lowerer::InsertLabel(const bool isHelper, IR::Instr *const insertBeforeInstr)
1488514989
{
1488614990
Assert(insertBeforeInstr);

‎lib/Backend/Lower.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,12 @@ class Lowerer
362362
void GenerateFastBrBReturn(IR::Instr * instr);
363363

364364
public:
365+
static IR::Instr *Lowerer::HoistIndirOffset(IR::Instr* instr, IR::IndirOpnd *indirOpnd, RegNum regNum);
366+
static IR::Instr *Lowerer::HoistIndirOffsetAsAdd(IR::Instr* instr, IR::IndirOpnd *orgOpnd, IR::Opnd *baseOpnd, int offset, RegNum regNum);
367+
static IR::Instr *Lowerer::HoistIndirIndexOpndAsAdd(IR::Instr* instr, IR::IndirOpnd *orgOpnd, IR::Opnd *baseOpnd, IR::Opnd *indexOpnd, RegNum regNum);
368+
static IR::Instr *Lowerer::HoistSymOffset(IR::Instr *instr, IR::SymOpnd *symOpnd, RegNum baseReg, uint32 offset, RegNum regNum);
369+
static IR::Instr *Lowerer::HoistSymOffsetAsAdd(IR::Instr* instr, IR::SymOpnd *orgOpnd, IR::Opnd *baseOpnd, int offset, RegNum regNum);
370+
365371
static IR::LabelInstr * InsertLabel(const bool isHelper, IR::Instr *const insertBeforeInstr);
366372

367373
static IR::Instr * InsertMove(IR::Opnd *dst, IR::Opnd *src, IR::Instr *const insertBeforeInstr, bool generateWriteBarrier = true);

‎lib/Backend/arm/LegalizeMD.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ void LegalizeMD::LegalizeIndirOffset(IR::Instr * instr, IR::IndirOpnd * indirOpn
266266

267267
if (indirOpnd->GetIndexOpnd() != NULL && offset != 0)
268268
{
269-
IR::Instr *addInstr = instr->HoistIndirOffset(indirOpnd, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
269+
IR::Instr *addInstr = Lowerer::HoistIndirOffset(instr, indirOpnd, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
270270
LegalizeMD::LegalizeInstr(addInstr, fPostRegAlloc);
271271
return;
272272
}
@@ -288,7 +288,7 @@ void LegalizeMD::LegalizeIndirOffset(IR::Instr * instr, IR::IndirOpnd * indirOpn
288288
}
289289

290290
// Offset is too large, so hoist it and replace it with an index, only valid for Thumb & Thumb2
291-
IR::Instr *addInstr = instr->HoistIndirOffset(indirOpnd, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
291+
IR::Instr *addInstr = Lowerer::HoistIndirOffset(instr, indirOpnd, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
292292
LegalizeMD::LegalizeInstr(addInstr, fPostRegAlloc);
293293
}
294294

@@ -334,7 +334,7 @@ void LegalizeMD::LegalizeSymOffset(
334334
}
335335

336336
IR::RegOpnd *baseOpnd = IR::RegOpnd::New(NULL, baseReg, TyMachPtr, instr->m_func);
337-
IR::Instr* instrAdd = instr->HoistSymOffsetAsAdd(symOpnd, baseOpnd, offset, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
337+
IR::Instr* instrAdd = Lowerer::HoistSymOffsetAsAdd(instr, symOpnd, baseOpnd, offset, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
338338
LegalizeMD::LegalizeInstr(instrAdd, fPostRegAlloc);
339339
return;
340340
}
@@ -351,7 +351,7 @@ void LegalizeMD::LegalizeSymOffset(
351351
}
352352
else
353353
{
354-
newInstr = instr->HoistSymOffset(symOpnd, baseReg, offset, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
354+
newInstr = Lowerer::HoistSymOffset(instr, symOpnd, baseReg, offset, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
355355
LegalizeMD::LegalizeInstr(newInstr, fPostRegAlloc);
356356
}
357357
}
@@ -621,14 +621,14 @@ void LegalizeMD::LegalizeIndirOpndForVFP(IR::Instr* insertInstr, IR::IndirOpnd *
621621
indexOpnd = newIndexOpnd;
622622
}
623623

624-
insertInstr->HoistIndirIndexOpndAsAdd(indirOpnd, baseOpnd, indexOpnd, fPostRegAlloc? SCRATCH_REG : RegNOREG);
624+
Lowerer::HoistIndirIndexOpndAsAdd(insertInstr, indirOpnd, baseOpnd, indexOpnd, fPostRegAlloc? SCRATCH_REG : RegNOREG);
625625
}
626626

627627
if (IS_CONST_UINT10((offset < 0? -offset: offset)))
628628
{
629629
return;
630630
}
631-
IR::Instr* instrAdd = insertInstr->HoistIndirOffsetAsAdd(indirOpnd, indirOpnd->GetBaseOpnd(), offset, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
631+
IR::Instr* instrAdd = Lowerer::HoistIndirOffsetAsAdd(insertInstr, indirOpnd, indirOpnd->GetBaseOpnd(), offset, fPostRegAlloc ? SCRATCH_REG : RegNOREG);
632632
LegalizeMD::LegalizeInstr(instrAdd, fPostRegAlloc);
633633
}
634634

‎lib/Backend/arm64/EncoderMD.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ void EncoderMD::CanonicalizeLea(IR::Instr * instr)
152152
indirOpnd->Free(this->m_func);
153153
}
154154
instr->m_opcode = Js::OpCode::ADD;
155+
Assert(instr->GetSrc1()->GetSize() == instr->GetSrc2()->GetSize());
155156
}
156157

157158
bool

0 commit comments

Comments
 (0)