[llvm-commits] [PATCH] Add single entry / single exit accessors.

[Andreas Simbuerger]

Am 14.01.2011 01:00, schrieb Tobias Grosser:
>>>> >>  -    BasicBlock *Pred = *PI;
>>>> >>  +    Pred = *PI;
>>>> >>         if (DT->getNode(Pred)&&   !contains(Pred)) {
>>>> >>  -      if (found) {
>>>> >>  -        isSimple = false;
>>>> >>  -        break;
>>>> >>  -      }
>>>> >>  -      found = true;
>>>> >>  +      if (ee)
>>>> >>  +        return 0;
>>>> >>  +
>>>> >>  +      ee = Pred;
>>> >  Can you combine the two if conditions?
>> I don't understand, as the predicate
>> (DT->getNode(Pred)&&  !contains(Pred)) has to be valid for
>> both statements inside the block, so I would have to recheck that in
>> an else branch after merging both if's? Perhaps I'm just not thinking
>> clearly, it's a bit late;-)
> 
> Don't mind. I just looked at the patch and did not see the assignment.
> 
>>>> >>  +bool Region::isSimple() const {
>>>> >>  +  return !(isTopLevelRegion())
>>>> >>  +&&   (getSingleEntryBlock() != 0)
>>>> >>  +&&   (getSingleExitBlock() != 0);
>>> >  I am not sure if my email has removed some spaces/tabs. But please
>>> >  recheck indentation.
>> I think so, the indentation is correct within my mail client. I attached
>> two versions of the patch
>>
>> without *.svn.patch was generated using git diff --no-prefix ... and
>> the other one was generated using git format-patch.
> The format-patch one is fine. I changed this slightly to fit into one
> line. I believe this makes it more readable.
> 
>> +  /// @brief Return the first block of this region's single entry edge,
>> +  ///        if existing.
>> +  ///
>> +  /// @return The BasicBlock starting this region's single entry edge,
>> +  ///         else NULL.
>> +  BasicBlock *getSingleEntryBlock() const;
>> +
>> +  /// @brief Return the first block of this region's single exit edge,
>> +  ///        if existing.
>> +  ///
>> +  /// @return The BasicBlock starting this region's single exit edge,
>> +  ///         else NULL.
>> +  BasicBlock *getSingleExitBlock() const;
> 
> You forgot to rename the functions in the header file.
> 
> I committed the patch after fixing two items mentioned above:
> 
> http://llvm.org/viewvc/llvm-project/?view=rev&revision=123410
> 

Sorry, they were changed but it seems they got mixed up while switching
branches :/
Thanks for fixing. Shouldn't format patches when it's late :-).


> Thanks for your contribution
> 
> Tobi