[PATCH] D127831: BasicBlockUtils: Add a new way for CreateControlFlowHub()

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

In D127831#3890092 <https://reviews.llvm.org/D127831#3890092>, @bcahoon wrote:

> In D127831#3887582 <https://reviews.llvm.org/D127831#3887582>, @foad wrote:
>
>> Why do we still need the "old way"? Does the "new way" generate worse code in some cases?
>
> Good question. The new way does require an additional compare instruction to test where control continues after the loop. One for each output edge. But, yes, there is a tradeoff here. I think it's worth assessing if the "new way" can replace the "old way" at some point. My thinking here is to be conservative and keep the "old way" for now as I'm a little hesitant to replace it without more performance data.

Right, I understand that using an i32 instead of an i1 means that you need extra icmp instructions in the IR, but does it actually cause more instructions in the final generated code (for AMDGPU)? If the i1 was stored in a general purpose register then you would need a cmp //anyway// to use it to control a conditional branch.


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

https://reviews.llvm.org/D127831