chakra-core / chakra-core/ChakraCore
Lowerer UseWithNewType() and Legalize() counter-intuitive behavior
- Dominant language
- JavaScript
- Stars
- 9.3k
- Forks
- 1.2k
- PR merge metrics
- No merged PRs in 30d
Description
Some SIMD.js x64test jshost failures were related to regAlloc generating bad code if casting to TyInt8. And I had to pin TyInt8 operands to a byteable register (#549).
What we want to do is cast reg2 to 1-byte then sign-extend:
```
IR::RegOpnd *reg2 = IR::RegOpnd::New(TyInt32, m_func);
IR::RegOpnd * tmp = IR::RegOpnd::New(TyInt8, m_func);
instr->InsertBefore(IR::Instr::New(Js::OpCode::MOV, tmp->UseWithNewType(TyInt32, m_func), reg2, m_func));
instr->InsertBefore(IR::Instr::New(Js::OpCode::MOVSX, reg2, tmp, m_func));
```
The issue is that UseWithNewType uses the original RegOpnd object if it's not already used by another instruction. Since the MOV is the first use, we inadvertently changed the type of tmp in MOVSX as well.
I suggest have UseWithNewType() always copy the opnd (or at least assert this is not the first use of the opnd).
We also tried this:
```
IR::RegOpnd *reg2 = IR::RegOpnd::New(TyInt32, m_func);
IR::RegOpnd * tmp = IR::RegOpnd::New(TyInt8, m_func);
instr->InsertBefore(IR::Instr::New(Js::OpCode::MOV, tmp, reg2, m_func));
instr->InsertBefore(IR::Instr::New(Js::OpCode::MOVSX, reg2, tmp, m_func));
```
The problem here is that we missed legalization of the MOV instruction, which should fix reg2 to be TyInt8. However, LowererMD::Legalize< verify=true >() doesn't assert due to the malformed code, but instead fixes it silently. The code is then valid only in debug mode, and breaks in test/release. I suggest we assert there.
```
template
void
LowererMD::Legalize(IR::Instr *const instr, bool fPostRegAlloc)
{
switch(instr->m_opcode)
{
...
case Js::OpCode::MOV:
{
else if (TySize[dstType] < TySize[srcType])
{
// if verify, we should assert(false) here.
instr->GetSrc1()->SetType(dst->GetType());
}
}
...
}
...
}
```
Contributor guide
Assessment
This issue has not been assessed yet.