[llvm] r179353 - Add a flag to align all basic blocks in the function.

[Nadav Rotem]

On Apr 11, 2013, at 6:04 PM, Chandler Carruth <chandlerc at google.com> wrote:

> On Thu, Apr 11, 2013 at 5:58 PM, Nadav Rotem <nrotem at apple.com> wrote:
> It aligns all blocks to a specific alignment. Do you prefer a different name ?
> 
> No, I understand why it is adding alignment, I'm asking why it also returns before actually doing any block layout... For example, this won't fix loop layout or anything else. It will *only* change the alignment.

I agree. Fixed in r179355.


> There is already a loop in the block layout code that does some alignment, I would naively expected this flag to be implemented by changing that logic.

I figured that the code will be more readable if I placed it outside of the BlockChain loop, but I don't mind either way. 

Thanks for the review. 

Nadav


> 
> On Apr 11, 2013, at 5:57 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
>> 
>> On Thu, Apr 11, 2013 at 5:48 PM, Nadav Rotem <nrotem at apple.com> wrote:
>> +  if (AlignAllBlock) {
>> +    // Align all of the blocks in the function to a specific alignment.
>> +    for (MachineFunction::iterator FI = F.begin(), FE = F.end();
>> +         FI != FE; ++FI)
>> +      FI->setAlignment(AlignAllBlock);
>> +    return true;
>> +  }
>> +
>>    buildCFGChains(F);
>> 
>> 
>> Huh? This turns of block layout altogether, which seems rather surprising for a flag called "align-all-blocks".
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130411/0fd53a8b/attachment.html>