================
@@ -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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to