<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 02/20/2015 03:46 PM, David Blaikie
wrote:<br>
</div>
<blockquote
cite="mid:CAENS6EsV9Xe6fjsP3Yqo7vjX3FBVM2EvYmm6p6g5jig15Ncj_w@mail.gmail.com"
type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, Feb 20, 2015 at 2:45 PM,
Philip Reames <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>
<div> <br>
<div>On 02/20/2015 02:24 PM, David Blaikie wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, Feb 20, 2015
at 2:05 PM, Philip Reames <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:listmail@philipreames.com"
target="_blank">listmail@philipreames.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author:
reames<br>
Date: Fri Feb 20 16:05:18 2015<br>
New Revision: 230068<br>
<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project?rev=230068&view=rev"
target="_blank">http://llvm.org/viewvc/llvm-project?rev=230068&view=rev</a><br>
Log:<br>
[RewriteStatepointsForGC] More style
cleanup [NFC]<br>
<br>
Use llvm_unreachable where appropriate,
use SmallVector where easy to do so,
introduce typedefs for planned type
migrations.<br>
<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp<br>
<br>
Modified:
llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=230068&r1=230067&r2=230068&view=diff"
target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=230068&r1=230067&r2=230068&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
(original)<br>
+++
llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
Fri Feb 20 16:05:18 2015<br>
@@ -96,10 +96,11 @@ namespace {<br>
// base relation will remain.
Internally, we add a mixture of the two<br>
// types, then update all the second type
to the first type<br>
typedef std::map<Value *, Value *>
DefiningValueMapTy;<br>
+typedef std::set<llvm::Value *>
StatepointLiveSetTy;<br>
<br>
struct
PartiallyConstructedSafepointRecord {<br>
/// The set of values known to be live
accross this safepoint<br>
- std::set<llvm::Value *> liveset;<br>
+ StatepointLiveSetTy liveset;<br>
<br>
/// Mapping from live pointers to a
base-defining-value<br>
DenseMap<llvm::Value *, llvm::Value
*> PointerToBase;<br>
@@ -283,7 +284,7 @@
analyzeParsePointLiveness(DominatorTree<br>
// Note: This output is used by
several of the test cases<br>
// The order of elemtns in a set is
not stable, put them in a vec and sort<br>
// by name<br>
- std::vector<Value *> temp;<br>
+ SmallVector<Value *, 64> temp;<br>
temp.insert(temp.end(),
liveset.begin(), liveset.end());<br>
std::sort(temp.begin(), temp.end(),
order_by_name);<br>
errs() << "Live Variables:\n";<br>
@@ -583,13 +584,14 @@ private:<br>
Value *base; // non null only if status
== base<br>
};<br>
<br>
+typedef std::map<Value *, PhiState>
ConflictStateMapTy;<br>
// Values of type PhiState form a
lattice, and this is a helper<br>
// class that implementes the meet
operation. The meat of the meet<br>
// operation is implemented in
MeetPhiStates::pureMeet<br>
class MeetPhiStates {<br>
public:<br>
// phiStates is a mapping from PHINodes
and SelectInst's to PhiStates.<br>
- explicit MeetPhiStates(const
std::map<Value *, PhiState>
&phiStates)<br>
+ explicit MeetPhiStates(const
ConflictStateMapTy &phiStates)<br>
: phiStates(phiStates) {}<br>
<br>
// Destructively meet the current
result with the base V. V can<br>
@@ -607,7 +609,7 @@ public:<br>
PhiState getResult() const { return
currentResult; }<br>
<br>
private:<br>
- const std::map<Value *, PhiState>
&phiStates;<br>
+ const ConflictStateMapTy
&phiStates;<br>
PhiState currentResult;<br>
<br>
/// Return a phi state for a base
defining value. We'll generate a new<br>
@@ -687,7 +689,7 @@ static Value
*findBasePointer(Value *I,<br>
// analougous to pessimistic data flow
and would likely lead to an<br>
// overall worse solution.<br>
<br>
- std::map<Value *, PhiState>
states;<br>
+ ConflictStateMapTy states;<br>
states[def] = PhiState();<br>
// Recursively fill in all phis &
selects reachable from the initial one<br>
// for which we don't already know a
definite base value for<br>
@@ -820,9 +822,8 @@ static Value
*findBasePointer(Value *I,<br>
v->getParent()->getParent()->getParent()->getContext(),
MDConst);<br>
basesel->setMetadata("is_base_value",
md);<br>
states[v] =
PhiState(PhiState::Conflict, basesel);<br>
- } else {<br>
- assert(false);<br>
- }<br>
+ } else<br>
+ llvm_unreachable("unknown
conflict type");<br>
</blockquote>
<div><br>
Rather than llvm_unreachable in an else -
take the 'if' and move it into an assert
instead?<br>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
This is an if-elseif-else chain. I could check both the
if and else-if conditions in an assert before hand, but
that seems less readable. This could also be arranged
as a switch, but then I'd loss the dyn_cast. </div>
</blockquote>
<div><br>
I was thinking of something simpler - changing the if
condition to an assert inside the else block:<br>
<br>
if (x)<br>
...<br>
else if (y)<br>
...<br>
else<br>
unreachable<br>
<br>
-><br>
<br>
if (x)<br>
...<br>
else<br>
assert (y)<br>
...<br>
<br>
In some cases, dyn_cast just becomes a cast and there's no
need for the assert at all (teh cast will assert) -
sometimes no need for a named variable if it's only used
once.<br>
<br>
I've committed a bunch of cleanup of unreachables in this
file, including this one, in <span
style="color:rgb(0,0,0)">r230094</span><br>
</div>
</div>
</div>
</div>
</blockquote>
I'm not a huge fan of that pattern, but I don't really care either.
:)<br>
<br>
Thanks for the cleanups!<br>
<blockquote
cite="mid:CAENS6EsV9Xe6fjsP3Yqo7vjX3FBVM2EvYmm6p6g5jig15Ncj_w@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> I'm open to any of
these, but given there's no good choice, I'd probably
stick with what's here unless you have a strong
preference. <br>
<div>
<div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
}<br>
}<br>
<br>
@@ -922,9 +923,8 @@ static Value
*findBasePointer(Value *I,<br>
}<br>
basesel->setOperand(i,
base);<br>
}<br>
- } else {<br>
- assert(false &&
"unexpected type");<br>
- }<br>
+ } else<br>
+ llvm_unreachable("unexpected
conflict type");<br>
}<br>
}<br>
<br>
@@ -1356,7 +1356,7 @@ static void
stablize_order(SmallVectorIm<br>
SmallVectorImpl<Value *>
&livevec) {<br>
assert(basevec.size() ==
livevec.size());<br>
<br>
- std::vector<name_ordering> temp;<br>
+ SmallVector<name_ordering, 64>
temp;<br>
for (size_t i = 0; i <
basevec.size(); i++) {<br>
name_ordering v;<br>
v.base = basevec[i];<br>
@@ -1654,9 +1654,8 @@ static void
insertUseHolderAfter(CallSit<br>
Func, Values, "",
invoke->getUnwindDest()->getFirstInsertionPt());<br>
holders.push_back(normal_holder);<br>
holders.push_back(unwind_holder);<br>
- } else {<br>
- assert(false &&
"Unsupported");<br>
- }<br>
+ } else<br>
+ llvm_unreachable("unsupported call
type");<br>
}<br>
<br>
static void findLiveReferences(<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a moz-do-not-send="true"
href="mailto:llvm-commits@cs.uiuc.edu"
target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a moz-do-not-send="true"
href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits"
target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</body>
</html>