[clang] [llvm] [AARCH64][Neon] switch to using bitcasts in arm_neon.h where appropriate (PR #127043)

[via llvm-commits]

================
@@ -8747,28 +8752,32 @@ Value *CodeGenFunction::EmitCommonNeonBuiltinExpr(
   return Builder.CreateBitCast(Result, ResultType, NameHint);
 }
 
-Value *CodeGenFunction::EmitAArch64CompareBuiltinExpr(
-    Value *Op, llvm::Type *Ty, const CmpInst::Predicate Fp,
-    const CmpInst::Predicate Ip, const Twine &Name) {
-  llvm::Type *OTy = Op->getType();
-
-  // FIXME: this is utterly horrific. We should not be looking at previous
-  // codegen context to find out what needs doing. Unfortunately TableGen
-  // currently gives us exactly the same calls for vceqz_f32 and vceqz_s32
-  // (etc).
-  if (BitCastInst *BI = dyn_cast<BitCastInst>(Op))
-    OTy = BI->getOperand(0)->getType();
-
-  Op = Builder.CreateBitCast(Op, OTy);
-  if (OTy->getScalarType()->isFloatingPointTy()) {
-    if (Fp == CmpInst::FCMP_OEQ)
-      Op = Builder.CreateFCmp(Fp, Op, Constant::getNullValue(OTy));
+Value *
+CodeGenFunction::EmitAArch64CompareBuiltinExpr(Value *Op, llvm::Type *Ty,
+                                               const CmpInst::Predicate Pred,
+                                               const Twine &Name) {
+
+  if (isa<FixedVectorType>(Ty)) {
+    // Vector types are cast to i8 vectors. Recover original type.
+    Op = Builder.CreateBitCast(Op, Ty);
+  }
+
+  if (CmpInst::isFPPredicate(Pred)) {
+    if (Pred == CmpInst::FCMP_OEQ)
+      Op = Builder.CreateFCmp(Pred, Op, Constant::getNullValue(Op->getType()));
     else
-      Op = Builder.CreateFCmpS(Fp, Op, Constant::getNullValue(OTy));
+      Op = Builder.CreateFCmpS(Pred, Op, Constant::getNullValue(Op->getType()));
   } else {
-    Op = Builder.CreateICmp(Ip, Op, Constant::getNullValue(OTy));
+    Op = Builder.CreateICmp(Pred, Op, Constant::getNullValue(Op->getType()));
   }
-  return Builder.CreateSExt(Op, Ty, Name);
+
+  llvm::Type *ResTy = Ty;
----------------
Lukacma wrote:

I believe this code is necessary because LLVM comparison instructions [return i1 vectors](https://llvm.org/docs/LangRef.html#id312), which need to be explicitly extended to i<elem_size> for correct behavior. As for why this part could be removed without immediate failures, I’m not entirely sure — LLVM doesn’t support implicit type casts, so the issue might be masked. It’s possible we’re missing tests that cover the full lowering path from C to assembly, which would otherwise reveal that the generated LLVM IR doesn’t map to any valid instruction.

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