[PATCH] D158328: [PowerPC] Merge rotate and clear into single instruction.

[Amy Kwan via Phabricator via llvm-commits]

amyk added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5105
+
+  SDValue Ops[] = {Val.getOperand(0), Val.getOperand(1), getI32Imm(MB, dl)};
+  CurDAG->SelectNodeTo(N, PPC::RLDCL, MVT::i64, Ops);
----------------
Maybe we can save `Val.getOperand(1)` separately since we're using it twice in this function.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5103
+
+  if (Val.getOperand(1).getOpcode() == ISD::Constant)
+    return false;
----------------
stefanp wrote:
> nemanjai wrote:
> > Why does this need to be constant?
> I'm trying to avoid matching the following kind of pattern:
> ```
> %2 = tail call i64 @llvm.fshl.i64(i64 %word, i64 %word, i64 23)
> %and1 = and i64 %2, 9223372036854775807
> ```
> The reason for this is that in the above case we can't use `RLDCL` but we have to use `RLDICL` instead. See the function `tworotates` in the test case.
> 
> Now, I spent a bit of time and I realize that this condition won't trigger because the above test case is caught earlier by the check in `tryBitPermutation`. However, I still want to have this here just in case the above changes or the ordering changes in the future. It's just for safety to make sure that we don't try to use an immediate in a place where we shouldn't be using one.
Can we add that documentation in the code as to why want it to be a constant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158328