[PATCH] D87145: [SelectionDAG] Remove an early-out from computeKnownBits for smin/smax

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/test/CodeGen/X86/masked_store_trunc_usat.ll:5199
+; SSE2-NEXT:    pminsw %xmm9, %xmm0
+; SSE2-NEXT:    pand %xmm8, %xmm0
 ; SSE2-NEXT:    packuswb %xmm1, %xmm0
----------------
foad wrote:
> RKSimon wrote:
> > regression?
> Wel, yes... It has spotted that the result of the pminsw is always negative, so rather than XOR with 0x8000 to flip (i.e. clear) the sign bit, it can AND with 0x7fff to clear the sign bit. But unfortunately that means materialising another constant.
> 
> I don't know where this XOR -> AND "optimization" happens, or whether it can be finessed.
> 
> The other regressions you pointed out below are basically the same issue.
> I don't know where this XOR -> AND "optimization" happens, or whether it can be finessed.

We have done this basically forever, in the XOR case in TargetLowering::SimplifyDemandedBits:
```
      // If one side is a constant, and all of the known set bits on the other
      // side are also set in the constant, turn this into an AND, as we know
      // the bits will be cleared.
      //    e.g. (X | C1) ^ C2 --> (X | C1) & ~C2 iff (C1&C2) == C2
```
It seems to me that this is just bad luck, that we transform X^0x8000 into X&0x7FFF, but that happens to regress code quality because now we can't share with another use of the constant 0x8000. Is there any systematic way of fixing this, e.g. by doing the reverse transformation once we know what constants are already available in registers? Or can I commit this patch even with a known regression like this? After all, I'm sure if I looked hard enough I could find another test that got better by luck instead of worse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87145