Commit bc1a66c7 by Nicolas Capens Committed by Nicolas Capens

Fix lowering and optimization of 64-bit absolute addresses

x86-64 does not support 64-bit immediates as absolute memory addresses. They have to be stored in a register, which can then be used as [base]. Previously we addressed this at the SubzeroReactor level by emitting a Bitcast from an Ice::Operand to an Ice::Variable, for which Subzero already supported 64-bit constants as input. This change implements X86OperandMem creation from a 64-bit constant operand by letting legalize() move it into a GPR and using it as the memory operand's base register. A Reactor unit test is added to exercise this. Another issue was that doLoadOpt() assumed all load instructions are candidates for fusing into a subsequent instruction which takes the result of the load. This isn't true when for 64-bit constant addresses an instruction to copy it into a register is inserted. For now this case is simply skipped. A future optimization could adjust the iterators properly so the load from [base] can be fused with the next instruction. Lastly, it is possible for a 64-bit constant to fit within a 32-bit immediate, in which case legalize() by default does not perform the copy into a GPR (note this is to allow moves and calls with 64-bit immediates, where they are legal), and simply returns the 64-bit constant. So we must not allow legalization to an immediate in this case. Note that while we could replace it with a 32-bit constant, it's rare for absolute addresses to fit in this range, and it would be non-deterministic which path is taken, so for consistency we don't perform this optimization. Bug: b/148272103 Change-Id: I5fcfa971dc93f2307202ee11619e84c65fe46188 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/52768Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAntonio Maiorano <amaiorano@google.com>
parent 6649bd02
......@@ -319,14 +319,6 @@ void Optimizer::eliminateLoadsFollowingSingleStore()
continue;
}
// TODO(b/148272103): InstLoad assumes that a constant ptr is an offset, rather than an
// absolute address. We need to make sure we don't replace a variable with a constant
// on this load.
if(llvm::isa<Ice::Constant>(storeValue))
{
continue;
}
replace(load, storeValue);
for(size_t i = 0; i < addressUses.loads.size(); i++)
......@@ -490,15 +482,6 @@ void Optimizer::optimizeStoresInSingleBasicBlock()
continue;
}
// TODO(b/148272103): InstLoad assumes that a constant ptr is an offset, rather than an
// absolute address. We need to make sure we don't replace a variable with a constant
// on this load.
if(llvm::isa<Ice::Constant>(storeValue))
{
unmatchedLoads = true;
continue;
}
replace(inst, storeValue);
}
}
......
......@@ -174,15 +174,6 @@ Ice::Variable *Call(Ice::Cfg *function, Ice::CfgNode *basicBlock, Return(fptr)(C
return Call(function, basicBlock, retTy, reinterpret_cast<void const *>(fptr), iceArgs, false);
}
// Returns a non-const variable copy of const v
Ice::Variable *createUnconstCast(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Constant *v)
{
Ice::Variable *result = function->makeVariable(v->getType());
Ice::InstCast *cast = Ice::InstCast::create(function, Ice::InstCast::Bitcast, result, v);
basicBlock->appendInst(cast);
return result;
}
Ice::Variable *createTruncate(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Operand *from, Ice::Type toType)
{
Ice::Variable *to = function->makeVariable(toType);
......@@ -193,14 +184,6 @@ Ice::Variable *createTruncate(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice:
Ice::Variable *createLoad(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Operand *ptr, Ice::Type type, unsigned int align)
{
// TODO(b/148272103): InstLoad assumes that a constant ptr is an offset, rather than an
// absolute address. We circumvent this by casting to a non-const variable, and loading
// from that.
if(auto *cptr = llvm::dyn_cast<Ice::Constant>(ptr))
{
ptr = sz::createUnconstCast(function, basicBlock, cptr);
}
Ice::Variable *result = function->makeVariable(type);
auto load = Ice::InstLoad::create(function, result, ptr, align);
basicBlock->appendInst(load);
......
......@@ -376,6 +376,23 @@ TEST(ReactorUnitTests, LoopAfterReturn)
EXPECT_EQ(result, 7);
}
TEST(ReactorUnitTests, ConstantPointer)
{
int c = 44;
FunctionT<int()> function;
{
Int x = *Pointer<Int>(ConstantPointer(&c));
Return(x);
}
auto routine = function(testName().c_str());
int result = routine();
EXPECT_EQ(result, 44);
}
// This test excercises the Optimizer::eliminateLoadsFollowingSingleStore() optimization pass.
// The three load operations for `y` should get eliminated.
// TODO(b/180665600): Check that the optimization took place.
......
......@@ -823,11 +823,15 @@ template <typename TraitsType> void TargetX86Base<TraitsType>::doLoadOpt() {
// Determine whether the current instruction is a Load instruction or
// equivalent.
if (auto *Load = llvm::dyn_cast<InstLoad>(CurInst)) {
// An InstLoad always qualifies.
LoadDest = Load->getDest();
constexpr bool DoLegalize = false;
LoadSrc = formMemoryOperand(Load->getLoadAddress(), LoadDest->getType(),
DoLegalize);
// An InstLoad qualifies unless it uses a 64-bit absolute address,
// which requires legalization to insert a copy to register.
// TODO(b/148272103): Fold these after legalization.
if (!Traits::Is64Bit || !llvm::isa<Constant>(Load->getLoadAddress())) {
LoadDest = Load->getDest();
constexpr bool DoLegalize = false;
LoadSrc = formMemoryOperand(Load->getLoadAddress(),
LoadDest->getType(), DoLegalize);
}
} else if (auto *Intrin = llvm::dyn_cast<InstIntrinsic>(CurInst)) {
// An AtomicLoad intrinsic qualifies as long as it has a valid memory
// ordering, and can be implemented in a single instruction (i.e., not
......@@ -8121,20 +8125,22 @@ TargetX86Base<TraitsType>::formMemoryOperand(Operand *Opnd, Type Ty,
auto *Offset = llvm::dyn_cast<Constant>(Opnd);
assert(Base || Offset);
if (Offset) {
// During memory operand building, we do not blind or pool the constant
// offset, we will work on the whole memory operand later as one entity
// later, this save one instruction. By turning blinding and pooling off,
// we guarantee legalize(Offset) will return a Constant*.
if (!llvm::isa<ConstantRelocatable>(Offset)) {
Offset = llvm::cast<Constant>(legalize(Offset));
}
if (llvm::isa<ConstantInteger64>(Offset)) {
// Memory operands cannot have 64-bit immediates, so they must be
// legalized into a register only.
Base = llvm::cast<Variable>(legalize(Offset, Legal_Reg));
Offset = nullptr;
} else {
Offset = llvm::cast<Constant>(legalize(Offset));
assert(llvm::isa<ConstantInteger32>(Offset) ||
llvm::isa<ConstantRelocatable>(Offset));
assert(llvm::isa<ConstantInteger32>(Offset) ||
llvm::isa<ConstantRelocatable>(Offset));
}
}
}
Mem = X86OperandMem::create(Func, Ty, Base, Offset);
}
// Do legalization, which contains pooling or do pooling.
return llvm::cast<X86OperandMem>(DoLegalize ? legalize(Mem) : Mem);
}
......
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