Skip to content

S
swiftshader
Commit 86dc7319 by Antonio Maiorano
Options

Subzero: proper fix for assert(Dest->hasReg())

Reverted the hack fix from bb222a10, and implemented a proper fix. The fix in TargetX86Base<TraitsType>::lowerCast (IceTargetLoweringX86BaseImpl.h) was suggested by Jim Stichnoth. The original problem is that we're trying movd an i32 to an i4i32, and since the target Variable is long-lived, it may not get a register allocated for it, and when that happens, we end up tripping the assert in InstX86Movd::emitIAS() that expects the destination to have been allocated a register. The solution that Jim suggested, and is implemented here, is to create a temporary, short-lived, variable to first movd into, which should guarantee a register target, since short-lived Variables usually get registers. Then we 'mov' the temporary register Variable to Dest, which *should* support moving the i4i32 register operand to an i32 memory operand. I said *should* above, because with the above fix, we now trip another assert in InstX86Mov::emitIAS (Mov, not Movd). The reason being that it doesn't actually support moving an i4i32 reg -> i32 memory operand. I added support for this as well (IceInstX86BaseImpl.h). Note that this assert only tripped when building with Om1 optimization level, since O2 ostensibly optimized out the mov call. Bug: b/145529686 Change-Id: I3c998a3e308838123cb415fcbf9f277113ac7d28 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/39068 Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com> Tested-by: 's avatarAntonio Maiorano <amaiorano@google.com>
parent afb4ebda
Showing with 10 additions and 5 deletions
third_party/subzero/src/IceInstX86Base.h
......@@ -1249,10 +1249,6 @@ template <typename TraitsType> struct InstImpl {
class InstX86Movd : public InstX86BaseUnaryopXmm<InstX86Base::Movd> {
public:
static InstX86Movd *create(Cfg *Func, Variable *Dest, Operand *Src) {
// TODO(amaiorano): This fixes assert(Dest->hasReg()) in emitIAS when
// there are no more registers left to allocate. Revisit this and fix it
// the right way. See b/145529686.
Dest->setMustHaveReg();
return new (Func->allocate<InstX86Movd>()) InstX86Movd(Func, Dest, Src);
}
......
third_party/subzero/src/IceInstX86BaseImpl.h
......@@ -2314,6 +2314,12 @@ void InstImpl<TraitsType>::InstX86Mov::emitIAS(const Cfg *Func) const {
Assembler *Asm = Func->getAssembler<Assembler>();
Asm->movss(SrcTy, StackAddr, Traits::getEncodedXmm(SrcVar->getRegNum()));
return;
} else if (isVectorType(SrcTy)) {
// Src must be a register
const auto *SrcVar = llvm::cast<Variable>(Src);
assert(SrcVar->hasReg());
Assembler *Asm = Func->getAssembler<Assembler>();
Asm->movups(StackAddr, Traits::getEncodedXmm(SrcVar->getRegNum()));
} else {
// Src can be a register or immediate.
assert(isScalarIntegerType(SrcTy));
......
third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
......@@ -3292,7 +3292,10 @@ void TargetX86Base<TraitsType>::lowerCast(const InstCast *Instr) {
// use v16i8 vectors.
assert(getFlags().getApplicationBinaryInterface() != ABI_PNaCl &&
"PNaCl only supports real 128-bit vectors");
_movd(Dest, legalize(Src0, Legal_Reg | Legal_Mem));
Operand *Src0RM = legalize(Src0, Legal_Reg | Legal_Mem);
Variable *T = makeReg(DestTy);
_movd(T, Src0RM);
_mov(Dest, T);
} else {
_movp(Dest, legalizeToReg(Src0));
}
......
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