[PATCH] D85479: [SVE][VLS] Don't combine logical AND.

Mon Aug 10 07:37:16 PDT 2020

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

A couple of recommendations to consider before landing the patch.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11111
+  // The combining code below works only for NEON vectors. In particular, it
+  // does not work for VLS SVE code generation when dealing with vectors wider
+  // than 128 bits.
----------------
Should people know what VLS means? Personally I think you can drop the reference since "It does not work for SVE when dealing with vectors larger than 128bits." should be clear enough.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11113
+  // than 128 bits.
+  if (VT.getSizeInBits() > 128)
+    return SDValue();
----------------
At some point we might have a genuine larger than NEON combine here at which point we should protect the NEON specific code with .is64BitVector || is128BitVector, but I guess right now this'll do.

================
Comment at: llvm/test/CodeGen/AArch64/vls-sve-512-and.ll:1
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve -aarch64-sve-vector-bits-min=512  -o - -asm-verbose=0 < %s | FileCheck %s
+
----------------
I'm still going to recommend again that you create tests using the sve-fixed-length- prefix so all relevant testing is kept together.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85479