[llvm] r230068 - [RewriteStatepointsForGC] More style cleanup [NFC]
Philip Reames
listmail at philipreames.com
Fri Feb 20 16:42:18 PST 2015
On 02/20/2015 03:46 PM, David Blaikie wrote:
>
>
> On Fri, Feb 20, 2015 at 2:45 PM, Philip Reames
> <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>
>
> On 02/20/2015 02:24 PM, David Blaikie wrote:
>>
>>
>> On Fri, Feb 20, 2015 at 2:05 PM, Philip Reames
>> <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>>
>> Author: reames
>> Date: Fri Feb 20 16:05:18 2015
>> New Revision: 230068
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=230068&view=rev
>> Log:
>> [RewriteStatepointsForGC] More style cleanup [NFC]
>>
>> Use llvm_unreachable where appropriate, use SmallVector where
>> easy to do so, introduce typedefs for planned type migrations.
>>
>>
>> Modified:
>> llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>>
>> Modified:
>> llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=230068&r1=230067&r2=230068&view=diff
>> ==============================================================================
>> ---
>> llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>> (original)
>> +++
>> llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>> Fri Feb 20 16:05:18 2015
>> @@ -96,10 +96,11 @@ namespace {
>> // base relation will remain. Internally, we add a mixture
>> of the two
>> // types, then update all the second type to the first type
>> typedef std::map<Value *, Value *> DefiningValueMapTy;
>> +typedef std::set<llvm::Value *> StatepointLiveSetTy;
>>
>> struct PartiallyConstructedSafepointRecord {
>> /// The set of values known to be live accross this safepoint
>> - std::set<llvm::Value *> liveset;
>> + StatepointLiveSetTy liveset;
>>
>> /// Mapping from live pointers to a base-defining-value
>> DenseMap<llvm::Value *, llvm::Value *> PointerToBase;
>> @@ -283,7 +284,7 @@ analyzeParsePointLiveness(DominatorTree
>> // Note: This output is used by several of the test cases
>> // The order of elemtns in a set is not stable, put them
>> in a vec and sort
>> // by name
>> - std::vector<Value *> temp;
>> + SmallVector<Value *, 64> temp;
>> temp.insert(temp.end(), liveset.begin(), liveset.end());
>> std::sort(temp.begin(), temp.end(), order_by_name);
>> errs() << "Live Variables:\n";
>> @@ -583,13 +584,14 @@ private:
>> Value *base; // non null only if status == base
>> };
>>
>> +typedef std::map<Value *, PhiState> ConflictStateMapTy;
>> // Values of type PhiState form a lattice, and this is a helper
>> // class that implementes the meet operation. The meat of
>> the meet
>> // operation is implemented in MeetPhiStates::pureMeet
>> class MeetPhiStates {
>> public:
>> // phiStates is a mapping from PHINodes and SelectInst's
>> to PhiStates.
>> - explicit MeetPhiStates(const std::map<Value *, PhiState>
>> &phiStates)
>> + explicit MeetPhiStates(const ConflictStateMapTy &phiStates)
>> : phiStates(phiStates) {}
>>
>> // Destructively meet the current result with the base V.
>> V can
>> @@ -607,7 +609,7 @@ public:
>> PhiState getResult() const { return currentResult; }
>>
>> private:
>> - const std::map<Value *, PhiState> &phiStates;
>> + const ConflictStateMapTy &phiStates;
>> PhiState currentResult;
>>
>> /// Return a phi state for a base defining value. We'll
>> generate a new
>> @@ -687,7 +689,7 @@ static Value *findBasePointer(Value *I,
>> // analougous to pessimistic data flow and would likely
>> lead to an
>> // overall worse solution.
>>
>> - std::map<Value *, PhiState> states;
>> + ConflictStateMapTy states;
>> states[def] = PhiState();
>> // Recursively fill in all phis & selects reachable from
>> the initial one
>> // for which we don't already know a definite base value for
>> @@ -820,9 +822,8 @@ static Value *findBasePointer(Value *I,
>> v->getParent()->getParent()->getParent()->getContext(),
>> MDConst);
>> basesel->setMetadata("is_base_value", md);
>> states[v] = PhiState(PhiState::Conflict, basesel);
>> - } else {
>> - assert(false);
>> - }
>> + } else
>> + llvm_unreachable("unknown conflict type");
>>
>>
>> Rather than llvm_unreachable in an else - take the 'if' and move
>> it into an assert instead?
> 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.
>
>
> I was thinking of something simpler - changing the if condition to an
> assert inside the else block:
>
> if (x)
> ...
> else if (y)
> ...
> else
> unreachable
>
> ->
>
> if (x)
> ...
> else
> assert (y)
> ...
>
> 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.
>
> I've committed a bunch of cleanup of unreachables in this file,
> including this one, in r230094
I'm not a huge fan of that pattern, but I don't really care either. :)
Thanks for the cleanups!
>
> 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.
>>
>> }
>> }
>>
>> @@ -922,9 +923,8 @@ static Value *findBasePointer(Value *I,
>> }
>> basesel->setOperand(i, base);
>> }
>> - } else {
>> - assert(false && "unexpected type");
>> - }
>> + } else
>> + llvm_unreachable("unexpected conflict type");
>> }
>> }
>>
>> @@ -1356,7 +1356,7 @@ static void stablize_order(SmallVectorIm
>> SmallVectorImpl<Value *> &livevec) {
>> assert(basevec.size() == livevec.size());
>>
>> - std::vector<name_ordering> temp;
>> + SmallVector<name_ordering, 64> temp;
>> for (size_t i = 0; i < basevec.size(); i++) {
>> name_ordering v;
>> v.base = basevec[i];
>> @@ -1654,9 +1654,8 @@ static void insertUseHolderAfter(CallSit
>> Func, Values, "",
>> invoke->getUnwindDest()->getFirstInsertionPt());
>> holders.push_back(normal_holder);
>> holders.push_back(unwind_holder);
>> - } else {
>> - assert(false && "Unsupported");
>> - }
>> + } else
>> + llvm_unreachable("unsupported call type");
>> }
>>
>> static void findLiveReferences(
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150220/a5dc0bca/attachment.html>
More information about the llvm-commits
mailing list