[PATCH] D57718: [PPC] Adjust the computed branch offset for the possible shorter distance

Guozhi Wei via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 19 14:32:02 PST 2019


Carrot added a comment.

In D57718#1402795 <https://reviews.llvm.org/D57718#1402795>, @nemanjai wrote:

> I can't help but feel like this patch adds further complication to an already excessively complicated function. To put things in perspective - this file is currently 282 lines and 210 of those lines are in a single function. Frankly, I think refactoring this into a few conceptual sections implemented in separate functions would go a long way.
>
> Seems to me that conceptually this pass does the following:
>
> - Renumber blocks
> - Compute the size of each block
> - Identify PC-relative branches and compute the branch distance
> - Convert PC-relative branches that branch too far into the "long branch sequence" (i.e. invert branch condition, convert to `bc+8, b`)
>
>   I understand if you don't want to do this refactoring and want to proceed with this patch as-is to solve the immediate problem. However, I thought I would bring this up since it seems like refactoring this would go a long way to making this readable.


I have similar feeling when I read this file at the first time. I will do the refactoring after commit this patch. I prefer to separate bug fixing and refactoring.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57718/new/

https://reviews.llvm.org/D57718





More information about the llvm-commits mailing list