Discern between load and store addresses
There were InstLoad::getSourceAddress() and InstStore::getAddr()
methods, which aren't very clear and consistently named. This change
replaces them with getLoadAddress() and getStoreAddress(), respectively.
This will also enable moving these methods to the Inst class to make
them available for SubVectorLoad and SubVectorStore intrinsics. While
these methods don't make sense for other instructions, note that
Inst::getSrc() already provides access to all operands and has to be
used with knowledge of the operand meaning and layout. So this only
provides a name to these operands, and it would stick out as a sore
thumb if used incorrectly.
Bug: b/179497998
Change-Id: I86b1201b8a1c611682f4f91541bdb49e17ef71a8
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/52530
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Reactor/Optimizer.cpp b/src/Reactor/Optimizer.cpp
index 768760f..27b14e3 100644
--- a/src/Reactor/Optimizer.cpp
+++ b/src/Reactor/Optimizer.cpp
@@ -630,7 +630,7 @@
if(auto *store = llvm::dyn_cast<Ice::InstStore>(instruction))
{
- return store->getAddr();
+ return store->getStoreAddress();
}
if(auto *storeSubVector = asStoreSubVector(instruction))
@@ -647,7 +647,7 @@
if(auto *load = llvm::dyn_cast<Ice::InstLoad>(instruction))
{
- return load->getSourceAddress();
+ return load->getLoadAddress();
}
if(auto *loadSubVector = asLoadSubVector(instruction))
diff --git a/third_party/subzero/src/IceASanInstrumentation.cpp b/third_party/subzero/src/IceASanInstrumentation.cpp
index 659fce1..4d249f8 100644
--- a/third_party/subzero/src/IceASanInstrumentation.cpp
+++ b/third_party/subzero/src/IceASanInstrumentation.cpp
@@ -363,7 +363,7 @@
void ASanInstrumentation::instrumentLoad(LoweringContext &Context,
InstLoad *Instr) {
- Operand *Src = Instr->getSourceAddress();
+ Operand *Src = Instr->getLoadAddress();
if (auto *Reloc = llvm::dyn_cast<ConstantRelocatable>(Src)) {
auto *NewLoad = InstLoad::create(Context.getNode()->getCfg(),
Instr->getDest(), instrumentReloc(Reloc));
@@ -373,7 +373,7 @@
}
Constant *Func =
Ctx->getConstantExternSym(Ctx->getGlobalString("__asan_check_load"));
- instrumentAccess(Context, Instr->getSourceAddress(),
+ instrumentAccess(Context, Instr->getLoadAddress(),
typeWidthInBytes(Instr->getDest()->getType()), Func);
}
@@ -381,15 +381,16 @@
InstStore *Instr) {
Operand *Data = Instr->getData();
if (auto *Reloc = llvm::dyn_cast<ConstantRelocatable>(Data)) {
- auto *NewStore = InstStore::create(
- Context.getNode()->getCfg(), instrumentReloc(Reloc), Instr->getAddr());
+ auto *NewStore =
+ InstStore::create(Context.getNode()->getCfg(), instrumentReloc(Reloc),
+ Instr->getStoreAddress());
Instr->setDeleted();
Context.insert(NewStore);
Instr = NewStore;
}
Constant *Func =
Ctx->getConstantExternSym(Ctx->getGlobalString("__asan_check_store"));
- instrumentAccess(Context, Instr->getAddr(),
+ instrumentAccess(Context, Instr->getStoreAddress(),
typeWidthInBytes(Instr->getData()->getType()), Func);
}
@@ -461,8 +462,8 @@
Cfg *Func = Context.getNode()->getCfg();
Context.setInsertPoint(Context.getCur());
for (InstStore *RzUnpoison : *ICE_TLS_GET_FIELD(LocalDtors)) {
- Context.insert(
- InstStore::create(Func, RzUnpoison->getData(), RzUnpoison->getAddr()));
+ Context.insert(InstStore::create(Func, RzUnpoison->getData(),
+ RzUnpoison->getStoreAddress()));
}
Context.advanceCur();
Context.advanceNext();
diff --git a/third_party/subzero/src/IceInst.cpp b/third_party/subzero/src/IceInst.cpp
index 5c9a5a3..556965c 100644
--- a/third_party/subzero/src/IceInst.cpp
+++ b/third_party/subzero/src/IceInst.cpp
@@ -873,7 +873,7 @@
Str << "store " << Ty << " ";
getData()->dump(Func);
Str << ", " << Ty << "* ";
- getAddr()->dump(Func);
+ getStoreAddress()->dump(Func);
Str << ", align " << typeAlignInBytes(Ty);
if (getRmwBeacon()) {
Str << ", beacon ";
diff --git a/third_party/subzero/src/IceInst.h b/third_party/subzero/src/IceInst.h
index bd38dd9..ac61f66 100644
--- a/third_party/subzero/src/IceInst.h
+++ b/third_party/subzero/src/IceInst.h
@@ -655,7 +655,7 @@
(void)Align;
return new (Func->allocate<InstLoad>()) InstLoad(Func, Dest, SourceAddr);
}
- Operand *getSourceAddress() const { return getSrc(0); }
+ Operand *getLoadAddress() const { return getSrc(0); }
bool isMemoryWrite() const override { return false; }
void dump(const Cfg *Func) const override;
static bool classof(const Inst *Instr) { return Instr->getKind() == Load; }
@@ -761,7 +761,7 @@
(void)Align;
return new (Func->allocate<InstStore>()) InstStore(Func, Data, Addr);
}
- Operand *getAddr() const { return getSrc(1); }
+ Operand *getStoreAddress() const { return getSrc(1); }
Operand *getData() const { return getSrc(0); }
Variable *getRmwBeacon() const;
void setRmwBeacon(Variable *Beacon);
diff --git a/third_party/subzero/src/IceTargetLoweringARM32.cpp b/third_party/subzero/src/IceTargetLoweringARM32.cpp
index 8d71e2e..aa75815 100644
--- a/third_party/subzero/src/IceTargetLoweringARM32.cpp
+++ b/third_party/subzero/src/IceTargetLoweringARM32.cpp
@@ -5483,7 +5483,7 @@
// A Load instruction can be treated the same as an Assign instruction, after
// the source operand is transformed into an OperandARM32Mem operand.
Type Ty = Load->getDest()->getType();
- Operand *Src0 = formMemoryOperand(Load->getSourceAddress(), Ty);
+ Operand *Src0 = formMemoryOperand(Load->getLoadAddress(), Ty);
Variable *DestLoad = Load->getDest();
// TODO(jvoung): handled folding opportunities. Sign and zero extension can
@@ -6168,7 +6168,7 @@
void TargetARM32::lowerStore(const InstStore *Instr) {
Operand *Value = Instr->getData();
- Operand *Addr = Instr->getAddr();
+ Operand *Addr = Instr->getStoreAddress();
OperandARM32Mem *NewAddr = formMemoryOperand(Addr, Value->getType());
Type Ty = NewAddr->getType();
diff --git a/third_party/subzero/src/IceTargetLoweringMIPS32.cpp b/third_party/subzero/src/IceTargetLoweringMIPS32.cpp
index bc8fc87..9876fe7 100644
--- a/third_party/subzero/src/IceTargetLoweringMIPS32.cpp
+++ b/third_party/subzero/src/IceTargetLoweringMIPS32.cpp
@@ -5211,7 +5211,7 @@
// A Load instruction can be treated the same as an Assign instruction, after
// the source operand is transformed into an OperandMIPS32Mem operand.
Type Ty = Instr->getDest()->getType();
- Operand *Src0 = formMemoryOperand(Instr->getSourceAddress(), Ty);
+ Operand *Src0 = formMemoryOperand(Instr->getLoadAddress(), Ty);
Variable *DestLoad = Instr->getDest();
auto *Assign = InstAssign::create(Func, DestLoad, Src0);
lowerAssign(Assign);
diff --git a/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h b/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
index b9570e7..98d9fba 100644
--- a/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
+++ b/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
@@ -765,8 +765,8 @@
// trigger, resulting in two loads and one store, which is worse than the
// original one load and one store. However, this is probably rare, and
// caching probably keeps it just as fast.
- if (!isSameMemAddressOperand<TraitsType>(Load->getSourceAddress(),
- Store->getAddr()))
+ if (!isSameMemAddressOperand<TraitsType>(Load->getLoadAddress(),
+ Store->getStoreAddress()))
continue;
Operand *ArithSrcFromLoad = Arith->getSrc(0);
Operand *ArithSrcOther = Arith->getSrc(1);
@@ -794,8 +794,9 @@
Store->setRmwBeacon(Beacon);
auto *BeaconDef = InstFakeDef::create(Func, Beacon);
Node->getInsts().insert(I3, BeaconDef);
- auto *RMW = InstX86FakeRMW::create(Func, ArithSrcOther, Store->getAddr(),
- Beacon, Arith->getOp());
+ auto *RMW =
+ InstX86FakeRMW::create(Func, ArithSrcOther, Store->getStoreAddress(),
+ Beacon, Arith->getOp());
Node->getInsts().insert(I3, RMW);
}
}
@@ -843,8 +844,8 @@
// An InstLoad always qualifies.
LoadDest = Load->getDest();
constexpr bool DoLegalize = false;
- LoadSrc = formMemoryOperand(Load->getSourceAddress(),
- LoadDest->getType(), DoLegalize);
+ 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
@@ -6065,7 +6066,7 @@
// it doesn't need another level of transformation.
Variable *DestLoad = Load->getDest();
Type Ty = DestLoad->getType();
- Operand *Src0 = formMemoryOperand(Load->getSourceAddress(), Ty);
+ Operand *Src0 = formMemoryOperand(Load->getLoadAddress(), Ty);
doMockBoundsCheck(Src0);
auto *Assign = InstAssign::create(Func, DestLoad, Src0);
lowerAssign(Assign);
@@ -6958,7 +6959,7 @@
template <typename TraitsType>
void TargetX86Base<TraitsType>::lowerStore(const InstStore *Instr) {
Operand *Value = Instr->getData();
- Operand *Addr = Instr->getAddr();
+ Operand *Addr = Instr->getStoreAddress();
X86OperandMem *NewAddr = formMemoryOperand(Addr, Value->getType());
doMockBoundsCheck(NewAddr);
Type Ty = NewAddr->getType();
@@ -6980,7 +6981,7 @@
template <typename TraitsType>
void TargetX86Base<TraitsType>::doAddressOptStore() {
auto *Instr = llvm::cast<InstStore>(Context.getCur());
- Operand *Addr = Instr->getAddr();
+ Operand *Addr = Instr->getStoreAddress();
Operand *Data = Instr->getData();
if (auto *OptAddr = computeAddressOpt(Instr, Data->getType(), Addr)) {
Instr->setDeleted();
