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

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:2196-2203
+  do {
+    SDValue O0 = CurrentOr->getOperand(0);
+    SDValue O1 = CurrentOr->getOperand(1);
+    if (O1.getOpcode() != ISD::AND)
+      return false;
+
+    switch (O0.getOpcode()) {
----------------
tsymalla wrote:
> nhaehnle wrote:
> > There is still a weird asymmetry here. Do you have a reason for treating the two operands differently? If not, you should treat them the same.
> The reason is that no transformation will happen if one of the operands doesn't fit the scheme. This is the case if
> 
> a) On the top level, a OR(AND, AND) pattern occurs (this is handled by ISel)
> b) the second operand is no AND in either case (handled by the top-level O1.getOpcode() check)
> c) A nested OR operand is used more than once - in that case, it cannot be optimized away
> d) Any of the AND operands is not a constant value, e. g. doesn't contribute to the mask.
> e) Any of the ANDs is being used more than once - in that case, it cannot be optimized away
> f) If none of the assumptions of the structure can be made, just return false.
> 
> It simplifies things if the operands are handled differently. Even if, for instance, the InsertBFIOp(O1) could theoretically be hoisted out, this complicates things at the BFI generation part later, because now the base and the order at the tail of the vector are swapped, making it impossible to just assign the back (base) to `NewNode` and drop it. So, this loop  basically relies on the state updated from the previous iteration and uses CurrentOr as controlling element.
> b) the second operand is no AND in either case (handled by the top-level O1.getOpcode() check)

What's the justification for this rule? Couldn't you have `N = (x1 & c1) | ((x2 & c2) | (x3 & c3))`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136432