[PATCH] D133840: AMDGPU: Add a pass to rewrite certain undef in PHI

[Ruiling, Song via Phabricator via llvm-commits]

ruiling added a comment.



> I understand that the block-split produces new opportunities for uniform phis, so we want to run this pass after "SIAnnotateControlFlow". But that does not automatically mean that we should assume that this is the *only* place it will ever run. By "work correctly", do you mean that the original handling of back-edges will produce incorrect IR? If not, that treatment of back-edges need not be removed.

By "work correctly", I mean the MachineIR translated from the LLVM IR should be correct. The handling of back-edge will produce IR that will be translated to incorrect MachineIR.

> Is there also an assumption that this pass works only with a structurized CFG? If both StructurizeCFG and SIAnnotateControlFlow are *necessary* for this pass to produce correct IR, then this needs to be document. If they are not, then this pass should be designed to work even without them, in order to preserve future opportunity. If loop backedges need extra work, at least we should document it in the code comments.

I have updated the comment for both issues. Please have a second look. Thanks very much for the careful review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133840