[PATCH] D77152: [SelectionDAG] Better legalization for FSHL and FSHR

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6163
+  unsigned RevOpcode = IsFSHL ? ISD::FSHR : ISD::FSHL;
+  if (!isOperationLegalOrCustom(Node->getOpcode(), VT) &&
+      isOperationLegalOrCustom(RevOpcode, VT)) {
----------------
bjope wrote:
> Is this transform even correct?
> 
> Assume we got `fshl 3, 2, 0`, then the result would be 3. But this code could turn it into `fshr 3, 2, (0-0)`, then the result would become 2.
Maybe one can limit the condition to

```
if (!isOperationLegalOrCustom(Node->getOpcode(), VT) &&
      isOperationLegalOrCustom(RevOpcode, VT) &&
      isPowerOf2_32(BW) && isNonZeroModBitWidth(Z, BW)) {
```

Although, isNonZeroModBitWidth currently allow undef values. And I wonder if it is allowed to turn `fshl X, Y, undef` into `fshr X, Y, (0-UNDEF)` , considering that we for example know that `fshl -1, 0, undef` can't be zero, but `fshr -1, 0, undef` can be zero. So one probably needs to restrict the condition further by adding a isNonZeroNonUndefModBitWidth helper and use that here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77152