[PATCH] D90162: [llvm][AArch64] Prevent spurious zero extention.

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli marked 6 inline comments as done.
fpetrogalli added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11314
+    // We care about the two different sign extension only if they are
+    // extending to the same EVT.
+    if (UseOne->getValueType(0) != UseTwo->getValueType(0))
----------------
samparker wrote:
> Why are we only caring about those cases, couldn't this generally help mixed types too?

I have removed this extra check. I originally added it with the intention to write a test case that was doing the following:
````
%x =  load i8
%A = zext %x to i64
... use %A
%cmp = sgt i8 %x, -1
```

But the output code with and without this extra check don't differ at all, so I think this check is not necessary.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5913
+      if (LHS.getOpcode() == ISD::SIGN_EXTEND_INREG) {
+        Mask = cast<VTSDNode>(LHS.getOperand(1))
+                   ->getVT()
----------------
peterwaller-arm wrote:
> I realise this problem is inherited from the existing code. "Mask" seems a confusing name given that it's a bit position. Perhaps "QueryBit", "TestBit", "BitToTest" or similar might be a better name?
> 
> (Applies above too)
I agree  with you. I have gone for: `s/Mask/SignBitPos/`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90162