[PATCH] D152911: [PowerPC] Remove extend between shift and and

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added a comment.

This patch makes sense to me. I just had one nit and one idea.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:15475
+    if ((Op1.getOpcode() != ISD::ZERO_EXTEND &&
+         Op1.getOpcode() != ISD::ANY_EXTEND) ||
+        !isa<ConstantSDNode>(Op2))
----------------
It may be possible to also make this safe for sign extend.
At this point you are using 
```
if(Imm >= maxUIntN(NarrowVT.getSizeInBits()))
  break;
```
to check if the immediate fits in the not yet extended range.
In the SIGN_EXTEND case you can do this by using 
```
if (Op1.getOpcode() == ISD::SIGN_EXTEND && Imm >= maxUIntN(NarrowVT.getSizeInBits() - 1))
      break;
```
When you subtract 1 from the range you know that the sign bit on whatever we extend is going to be zero (because it is for the constant and we are doing an AND) so the sign extend becomes a zero extend and everything is fine.

I'm not 100% about this idea but it might work.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:15482
+        NarrowOp.getOpcode() != ISD::ROTR &&
+        NarrowOp.getOpcode() != ISD::FSHL && NarrowOp.getOpcode() != ISD::FSHR)
+      break;
----------------
nit:
```
NarrowOp.getOpcode()
```
is used a lot in this if statement. You may want to pull it out.
```
unsigned NarrowOpcode = NarrowOp.getOpcode();
if (NarrowOpcode != ISD::SHL && ...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152911