-
Notifications
You must be signed in to change notification settings - Fork 183
Multiple RHS Support #367
base: main
Are you sure you want to change the base?
Multiple RHS Support #367
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,8 @@ class BaseSolver : public Solver { | |
|
|
||
| std::error_code infer(const BlockPCs &BPCs, | ||
| const std::vector<InstMapping> &PCs, | ||
| Inst *LHS, Inst *&RHS, InstContext &IC) override { | ||
| Inst *LHS, std::vector<Inst *> &RHSs, | ||
| InstContext &IC) override { | ||
| std::error_code EC; | ||
|
|
||
| /* | ||
|
|
@@ -103,7 +104,7 @@ class BaseSolver : public Solver { | |
| if (EC) | ||
| return EC; | ||
| if (!IsSat) { | ||
| RHS = I; | ||
| RHSs.emplace_back(I); | ||
| return EC; | ||
| } | ||
| } | ||
|
|
@@ -142,7 +143,7 @@ class BaseSolver : public Solver { | |
| if (EC) | ||
| return EC; | ||
| if (!IsSat) { | ||
| RHS = Const; | ||
| RHSs.emplace_back(Const); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| return EC; | ||
| } | ||
| } | ||
|
|
@@ -188,7 +189,7 @@ class BaseSolver : public Solver { | |
| if (EC) | ||
| return EC; | ||
| if (!SmallQueryIsSat) { | ||
| RHS = I; | ||
| RHSs.emplace_back(I); | ||
| break; | ||
| } | ||
| } | ||
|
|
@@ -214,18 +215,21 @@ class BaseSolver : public Solver { | |
| if(SMTSolver->supportsModels()) { | ||
| if (EnableExhaustiveSynthesis) { | ||
| ExhaustiveSynthesis ES; | ||
| EC = ES.synthesize(SMTSolver.get(), BPCs, PCs, LHS, RHS, IC, Timeout); | ||
| if (EC || RHS) | ||
| EC = ES.synthesize(SMTSolver.get(), BPCs, PCs, LHS, RHSs, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did you intend to change this line? was it too long?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RHS is changed to RHSs, and it is too long so I wrapped. |
||
| IC, Timeout); | ||
| if (EC || !RHSs.empty()) | ||
| return EC; | ||
| } else if (InferInsts) { | ||
| InstSynthesis IS; | ||
| Inst *RHS; | ||
| EC = IS.synthesize(SMTSolver.get(), BPCs, PCs, LHS, RHS, IC, Timeout); | ||
| RHSs.emplace_back(RHS); | ||
| if (EC || RHS) | ||
| return EC; | ||
| } | ||
| } | ||
|
|
||
| RHS = 0; | ||
| RHSs.clear(); | ||
| return EC; | ||
| } | ||
|
|
||
|
|
@@ -281,16 +285,17 @@ class MemCachingSolver : public Solver { | |
|
|
||
| std::error_code infer(const BlockPCs &BPCs, | ||
| const std::vector<InstMapping> &PCs, | ||
| Inst *LHS, Inst *&RHS, InstContext &IC) override { | ||
| Inst *LHS, std::vector<Inst *> &RHSs, | ||
| InstContext &IC) override { | ||
| ReplacementContext Context; | ||
| std::string Repl = GetReplacementLHSString(BPCs, PCs, LHS, Context); | ||
| const auto &ent = InferCache.find(Repl); | ||
| if (ent == InferCache.end()) { | ||
| ++MemMissesInfer; | ||
| std::error_code EC = UnderlyingSolver->infer(BPCs, PCs, LHS, RHS, IC); | ||
| std::error_code EC = UnderlyingSolver->infer(BPCs, PCs, LHS, RHSs, IC); | ||
| std::string RHSStr; | ||
| if (!EC && RHS) { | ||
| RHSStr = GetReplacementRHSString(RHS, Context); | ||
| if (!EC && !RHSs.empty()) { | ||
| RHSStr = GetReplacementRHSString(RHSs.front(), Context); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems weird to take the first element here, is this a placeholder for something different?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR does not support cache dumping yet, so only print the first RHS in the list. I'll add support for cache dumping soon.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say don't do the caching part of this yet, until we're sure we want it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, should be possible to prototype without caching. |
||
| } | ||
| InferCache.emplace(Repl, std::make_pair(EC, RHSStr)); | ||
| return EC; | ||
|
|
@@ -299,12 +304,12 @@ class MemCachingSolver : public Solver { | |
| std::string ES; | ||
| StringRef S = ent->second.second; | ||
| if (S == "") { | ||
| RHS = 0; | ||
| RHSs.clear(); | ||
| } else { | ||
| ParsedReplacement R = ParseReplacementRHS(IC, "<cache>", S, Context, ES); | ||
| if (ES != "") | ||
| return std::make_error_code(std::errc::protocol_error); | ||
| RHS = R.Mapping.RHS; | ||
| RHSs.emplace_back(R.Mapping.RHS); | ||
| } | ||
| return ent->second.first; | ||
| } | ||
|
|
@@ -351,7 +356,8 @@ class ExternalCachingSolver : public Solver { | |
|
|
||
| std::error_code infer(const BlockPCs &BPCs, | ||
| const std::vector<InstMapping> &PCs, | ||
| Inst *LHS, Inst *&RHS, InstContext &IC) override { | ||
| Inst *LHS, std::vector<Inst *> &RHSs, | ||
| InstContext &IC) override { | ||
| ReplacementContext Context; | ||
| std::string LHSStr = GetReplacementLHSString(BPCs, PCs, LHS, Context); | ||
| if (LHSStr.length() > MaxLHSSize) | ||
|
|
@@ -360,26 +366,26 @@ class ExternalCachingSolver : public Solver { | |
| if (KV->hGet(LHSStr, "result", S)) { | ||
| ++ExternalHits; | ||
| if (S == "") { | ||
| RHS = 0; | ||
| RHSs.clear(); | ||
| } else { | ||
| std::string ES; | ||
| ParsedReplacement R = ParseReplacementRHS(IC, "<cache>", S, Context, ES); | ||
| if (ES != "") | ||
| return std::make_error_code(std::errc::protocol_error); | ||
| RHS = R.Mapping.RHS; | ||
| RHSs.emplace_back(R.Mapping.RHS); | ||
| } | ||
| return std::error_code(); | ||
| } else { | ||
| ++ExternalMisses; | ||
| if (NoInfer) { | ||
| RHS = 0; | ||
| RHSs.clear(); | ||
| KV->hSet(LHSStr, "result", ""); | ||
| return std::error_code(); | ||
| } | ||
| std::error_code EC = UnderlyingSolver->infer(BPCs, PCs, LHS, RHS, IC); | ||
| std::error_code EC = UnderlyingSolver->infer(BPCs, PCs, LHS, RHSs, IC); | ||
| std::string RHSStr; | ||
| if (!EC && RHS) { | ||
| RHSStr = GetReplacementRHSString(RHS, Context); | ||
| if (!EC && !RHSs.empty()) { | ||
| RHSStr = GetReplacementRHSString(RHSs.front(), Context); | ||
| } | ||
| KV->hSet(LHSStr, "result", RHSStr); | ||
| return EC; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to return here, or try to find more RHSs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are both constant synthesis, why we wants to collect multiple RHSs if a constant fit the LHS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, it depends on why we want multiple RHSs. for manasij's purposes a constant is always good, but for building a cost model I'd like as many alternatives as possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping me if you need this.