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

Wed Oct 28 07:34:33 PDT 2020

peterwaller-arm added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11282

+  // Finds the pattern that realize the zero extension inreg for
+  // illegal values, which is rendered with an and instruction with a
----------------
s/that realize/implementing/ or "that realizes"  or "that implements".

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5915
+                   .getSizeInBits()
+                   .getFixedSize() -
+               1;
----------------
s/getSizeInBits().getFixedSize()/getFixedSizeInBits()/?

(Applies above too)

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:5913
+      if (LHS.getOpcode() == ISD::SIGN_EXTEND_INREG) {
+        Mask = cast<VTSDNode>(LHS.getOperand(1))
+                   ->getVT()
----------------
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)

================
Comment at: llvm/test/CodeGen/AArch64/zext-and-signed-compare.ll:2
+; RUN: llc -mtriple aarch64-linux-gnu -o -  -asm-verbose=0 < %s | FileCheck %s
+
+define i32 @f_i32_i8(i8* %p) nounwind {
----------------
Could do with a comment to explain the purpose of the test.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90162