[LLVMugs][Bug 20122] Patch ready
Hal Finkel
hfinkel at anl.gov
Tue Jul 1 04:21:45 PDT 2014
----- Original Message -----
> From: "wuhui1973" <wuhui1973 at 163.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Tuesday, July 1, 2014 4:57:06 AM
> Subject: Re:Re: [LLVMugs][Bug 20122] Patch ready
>
>
>
>
> Hello Hal:
>
>
> I found this comment about the assertion:
>
>
>
> // In theory, this is combinatorial. In practice, it needs to be
> bounded
> // by a small number of sets for regpressure to be efficient.
> // If the assert is hit, we need to implement pruning.
> assert(Idx < (2*NumRegUnitSubSets) && "runaway unit set inference");
>
> Seems the author had forseen this problem, and he (she) suggested to
> prune the unitset.
> I try to prune the unitset by pruneUnitSets if it exceeds the limit,
> but not succeed - the set is not reduced.
>
>
> I am now trying to open the debug output available in the code and
> see what's happening.
> Just a question: How to enable DEBUG macro in this source, which is
> run by tablegen?
If you run with the -debug command-line argument you should get the debugging output.
Thanks again,
Hal
>
>
> However, this error can be avoid, just using assert(Idx <
> (3*NumRegUnitSubSets) && "runaway unit set inference");
>
>
> Regards & Thanks At 2014-07-01 05:42:20, "Hal Finkel" <
> hfinkel at anl.gov > wrote:
> >Hi,
> >
> >With this patch applied, building LLVM's AMDGPU target fails:
> >
> >/src/llvm-trunk/utils/TableGen/CodeGenRegisters.cpp:1588: void
> >llvm::CodeGenRegBank::computeRegUnitSets(): Assertion `Idx <
> >(2*NumRegUnitSubSets) && "runaway unit set inference"' failed.
> >
> >Maybe the threshold in the assert just needs to be loosened, but
> >we'd need to understand what is going on here.
> >
> > -Hal
> >
> >----- Original Message -----
> >> From: "wuhui1973" < wuhui1973 at 163.com >
> >> To: llvm-commits at cs.uiuc.edu >> Sent: Sunday, June 29, 2014
> >> 11:08:40 PM
> >> Subject: [LLVMugs][Bug 20122] Patch ready
> >>
> >>
> >>
> >>
> >>
> >> [ reply ] [-] Description hui wu 2014-06-24 21:35:52 CDT In
> >> function
> >> CodeGenRegBank::computeRegUnitSets, below code fragment is used to
> >> calculate the transitive closure of set union of intersecting
> >> sets.
> >>
> >> // Iterate over all unit sets, including new ones added by this
> >> loop.
> >> unsigned NumRegUnitSubSets = RegUnitSets.size();
> >> for (unsigned Idx = 0, EndIdx = RegUnitSets.size(); Idx !=
> >> EndIdx;
> >> ++Idx) {
> >> // In theory, this is combinatorial. In practice, it needs to
> >> be
> >> bounded
> >> // by a small number of sets for regpressure to be efficient.
> >> // If the assert is hit, we need to implement pruning.
> >> assert(Idx < (2*NumRegUnitSubSets) && "runaway unit set
> >> inference");
> >>
> >> // Compare new sets with all original classes.
> >> for (unsigned SearchIdx = (Idx >= NumRegUnitSubSets) ? 0 :
> >> Idx+1;
> >> SearchIdx != EndIdx; ++SearchIdx) {
> >> std::set<unsigned> Intersection;
> >> std::set_intersection(RegUnitSets[Idx].Units.begin(),
> >> RegUnitSets[Idx].Units.end(),
> >> RegUnitSets[SearchIdx].Units.begin(),
> >> RegUnitSets[SearchIdx].Units.end(),
> >> std::inserter(Intersection,
> >> Intersection.begin()));
> >> if (Intersection.empty())
> >> continue;
> >>
> >> // Speculatively grow the RegUnitSets to hold the new set.
> >> RegUnitSets.resize(RegUnitSets.size() + 1);
> >> RegUnitSets.back().Name =
> >> RegUnitSets[Idx].Name + "+" + RegUnitSets[SearchIdx].Name;
> >>
> >> std::set_union(RegUnitSets[Idx].Units.begin(),
> >> RegUnitSets[Idx].Units.end(),
> >> RegUnitSets[SearchIdx].Units.begin(),
> >> RegUnitSets[SearchIdx].Units.end(),
> >> std::inserter(RegUnitSets.back().Units,
> >> RegUnitSets.back().Units.begin()));
> >>
> >> // Find an existing RegUnitSet, or add the union to the
> >> unique
> >> sets.
> >> std::vector<RegUnitSet>::const_iterator SetI =
> >> findRegUnitSet(RegUnitSets, RegUnitSets.back());
> >> if (SetI != llvm::prior(RegUnitSets.end()))
> >> RegUnitSets.pop_back();
> >> }
> >> }
> >>
> >> However, the use of EndIdx like that is not correct, it makes the
> >> for
> >> loop exiting too early without handling the new added sets. The
> >> correct way should be:
> >>
> >> // Iterate over all unit sets, including new ones added by this
> >> loop.
> >> unsigned NumRegUnitSubSets = RegUnitSets.size();
> >> for (unsigned Idx = 0; Idx != RegUnitSets.size(); ++Idx) {
> >> // In theory, this is combinatorial. In practice, it needs to
> >> be
> >> bounded
> >> // by a small number of sets for regpressure to be efficient.
> >> // If the assert is hit, we need to implement pruning.
> >> assert(Idx < (2*NumRegUnitSubSets) && "runaway unit set
> >> inference");
> >>
> >> // Compare new sets with all original classes.
> >> for (unsigned SearchIdx = (Idx >= NumRegUnitSubSets) ? 0 :
> >> Idx+1;
> >> SearchIdx != RegUnitSets.size(); ++SearchIdx) {
> >> std::set<unsigned> Intersection;
> >> std::set_intersection(RegUnitSets[Idx].Units.begin(),
> >> RegUnitSets[Idx].Units.end(),
> >> RegUnitSets[SearchIdx].Units.begin(),
> >> RegUnitSets[SearchIdx].Units.end(),
> >> std::inserter(Intersection,
> >> Intersection.begin()));
> >> if (Intersection.empty())
> >> continue;
> >>
> >> // Speculatively grow the RegUnitSets to hold the new set.
> >> RegUnitSets.resize(RegUnitSets.size() + 1);
> >> RegUnitSets.back().Name =
> >> RegUnitSets[Idx].Name + "+" + RegUnitSets[SearchIdx].Name;
> >>
> >> std::set_union(RegUnitSets[Idx].Units.begin(),
> >> RegUnitSets[Idx].Units.end(),
> >> RegUnitSets[SearchIdx].Units.begin(),
> >> RegUnitSets[SearchIdx].Units.end(),
> >> std::inserter(RegUnitSets.back().Units,
> >> RegUnitSets.back().Units.begin()));
> >>
> >> // Find an existing RegUnitSet, or add the union to the
> >> unique
> >> sets.
> >> std::vector<RegUnitSet>::const_iterator SetI =
> >> findRegUnitSet(RegUnitSets, RegUnitSets.back());
> >> if (SetI != llvm::prior(RegUnitSets.end()))
> >> RegUnitSets.pop_ back();
> >> }
> >> }
> >>
> >> [ reply ] [-] Comment 1 Hal Finkel 2014-06-25 05:02:27 CDT Thanks
> >> for
> >> filing this bug; do you know if this is still a problem in llvm
> >> trunk? If so, the best way to provide this fix is to send a patch
> >> to
> >> the llvm-commits mailing list. That is how we normally review
> >> proposed patches.
> >> http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch
> >> Also, if you know how to generate a test that triggers the
> >> incorrect
> >> behavior, that would be great!
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu >>
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> >
> >--
> >Hal Finkel
> >Assistant Computational Scientist
> >Leadership Computing Facility
> >Argonne National Laboratory
>
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list