Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(22)

Unified Diff: src/IceTargetLoweringX86BaseImpl.h

Issue 1904233002: Subzero: Fix over-aggressive bool folding. (Closed) Base URL: https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Patch Set: Hack for pure virtual method Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/IceIntrinsics.cpp ('k') | tests_lit/llvm2ice_tests/bool-folding.ll » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/IceTargetLoweringX86BaseImpl.h
diff --git a/src/IceTargetLoweringX86BaseImpl.h b/src/IceTargetLoweringX86BaseImpl.h
index 4927e170598d69eded4aa851733b30ae89b114c8..d106280f542315c242fa0d3c9be0dd98709dfaf6 100644
--- a/src/IceTargetLoweringX86BaseImpl.h
+++ b/src/IceTargetLoweringX86BaseImpl.h
@@ -151,6 +151,7 @@ private:
return Element != Producers.end() && Element->second.Instr != nullptr;
}
void setInvalid(SizeT VarNum) { Producers[VarNum].Instr = nullptr; }
+ void invalidateProducersOnStore(const Inst *Instr);
/// Producers maps Variable::Number to a BoolFoldingEntry.
CfgUnorderedMap<SizeT, BoolFoldingEntry<Traits>> Producers;
};
@@ -252,10 +253,12 @@ bool BoolFolding<Traits>::isValidFolding(
template <typename Traits> void BoolFolding<Traits>::init(CfgNode *Node) {
Producers.clear();
for (Inst &Instr : Node->getInsts()) {
+ if (Instr.isDeleted())
+ continue;
+ invalidateProducersOnStore(&Instr);
// Check whether Instr is a valid producer.
Variable *Var = Instr.getDest();
- if (!Instr.isDeleted() // only consider non-deleted instructions
- && Var // only instructions with an actual dest var
+ if (Var // only consider instructions with an actual dest var
&& Var->getType() == IceType_i1 // only bool-type dest vars
&& getProducerKind(&Instr) != PK_None) { // white-listed instructions
Producers[Var->getIndex()] = BoolFoldingEntry<Traits>(&Instr);
@@ -338,6 +341,43 @@ void BoolFolding<Traits>::dump(const Cfg *Func) const {
}
}
+/// If the given instruction has potential memory side effects (e.g. store, rmw,
+/// or a call instruction with potential memory side effects), then we must not
+/// allow a pre-store Producer instruction with memory operands to be folded
+/// into a post-store Consumer instruction. If this is detected, the Producer
+/// is invalidated.
+///
+/// We use the Producer's IsLiveOut field to determine whether any potential
+/// Consumers come after this store instruction. The IsLiveOut field is
+/// initialized to true, and BoolFolding::init() sets IsLiveOut to false when it
+/// sees the variable's definitive last use (indicating the variable is not in
+/// the node's live-out set). Thus if we see here that IsLiveOut is false, we
+/// know that there can be no consumers after the store, and therefore we know
+/// the folding is safe despite the store instruction.
+template <typename Traits>
+void BoolFolding<Traits>::invalidateProducersOnStore(const Inst *Instr) {
+ if (!Instr->isMemoryWrite())
+ return;
+ for (auto &ProducerPair : Producers) {
+ if (!ProducerPair.second.IsLiveOut)
+ continue;
+ Inst *PInst = ProducerPair.second.Instr;
+ if (PInst == nullptr)
+ continue;
+ bool HasMemOperand = false;
+ const SizeT SrcSize = PInst->getSrcSize();
+ for (SizeT I = 0; I < SrcSize; ++I) {
+ if (llvm::isa<typename Traits::X86OperandMem>(PInst->getSrc(I))) {
+ HasMemOperand = true;
+ break;
+ }
+ }
+ if (!HasMemOperand)
+ continue;
+ setInvalid(ProducerPair.first);
+ }
+}
+
template <typename TraitsType>
void TargetX86Base<TraitsType>::initNodeForLowering(CfgNode *Node) {
FoldingInfo.init(Node);
« no previous file with comments | « src/IceIntrinsics.cpp ('k') | tests_lit/llvm2ice_tests/bool-folding.ll » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698