[LLVMbugs] [Bug 20122] New: A code flaw found in function CodeGenRegBank::computeRegUnitSets

[bugzilla-daemon at llvm.org]

http://llvm.org/bugs/show_bug.cgi?id=20122

            Bug ID: 20122
           Summary: A code flaw found in function
                    CodeGenRegBank::computeRegUnitSets
           Product: new-bugs
           Version: 3.3
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P
         Component: new bugs
          Assignee: unassignedbugs at nondot.org
          Reporter: wuhui1973 at 163.com
                CC: llvmbugs at cs.uiuc.edu
    Classification: Unclassified

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();
    }
  }

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20140625/76ec00ed/attachment.html>