[clang] [CIR][AArch64] Complete upstream vector saturating shift left NEON builtins (PR #221507)

[Andrzej Warzyński via cfe-commits]

================
@@ -319,14 +319,24 @@ deriveNeonSISDIntrinsicOperandTypes(CIRGenFunction &cgf, unsigned modifier,
   // that has the same scalar type as arg0. Checking the ICE bitmap prevents
   // an i32 immediate from being vectorized when it has the same type as a
   // data operand (e.g. vqshrns_n_s32).
+  //
+  // Exception: the `_n_` scalar saturating shift-left builtins (vqshlb_n_s8,
+  // vqshlh_n_u16, vqshlub_n_s8, ...) lower to aarch64.neon.{s,u}qshl /
+  // sqshlu, whose second operand is `LLVMMatchType<0>` - a vector, not a
+  // scalar. Their shape is a single data operand followed by the immediate,
+  // so widen that trailing immediate too. Narrowing shifts
+  // (`ArgAsWidenedRetType`) keep a scalar i32 immediate.
+  const bool widenTrailingImm = vecArgTy && ops.size() == 2 &&
+                                !(modifier & ArgAsWidenedRetType) &&
+                                (iceArguments & (1U << 1));
----------------
banach-space wrote:

To avoid this special-casing, you could add another flag in `AArch64CodeGenUtils.h` and rely on that instead. I would still preserve some bits of this comment - it's very helpful - but perhaps move it to the header. 

In general, we are being penalised by the fact that we don't have access to the LLVM intrinsics at this level (as is the case for the classical code-gen). In cases like this (i.e. where the overall design is too weak) its good to document the issue in more "global" locations like the header file. Ideally, we would unify both implementations to rely on the flags, though it's a relatively low priority (patches are welcome!).

The suggested updated is a nice-to-have :)

https://github.com/llvm/llvm-project/pull/221507