[PATCH] D136432: [AMDGPU] Combine BFI instructions.

[Thomas Symalla via Phabricator via llvm-commits]

tsymalla added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:2194
+  SDNode *CurrentOr = N;
+  while (CurrentOr && CurrentOr->hasOneUse()) {
+    SDValue O0 = CurrentOr->getOperand(0);
----------------
foad wrote:
> tsymalla wrote:
> > foad wrote:
> > > The top level OR can have multiple uses.
> > Yes. You are right. Thanks.
> This is not the right place to put the hasOneUse check anyway. What would happen if the check failed? You would break out of the loop, and then generate wrong code.
> 
> Also how can the check for `CurrentOr` ever fail?
The right way to do so might be to just skip the whole function (to return false) if the OR gets used multiple times. Previously, it was possible for the CurrentOr check to fail, this is just a remnant. It currently works because the terminating condition for the loop is inside the non-OR handling branches of the main if which resides inside the loop. So either it continues looking for appropriate ORs or terminates once it found the final OR, AND, AND sequence. I'll simplify the whole thing and put another diff up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136432