[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'm not completely happy with using EmitPointerWithAlignment here... but I guess it's the same thing we do for 32-bit ARM, so it must be mostly usable in practice.  I'd like to see some tests in clang/test/CodeGen/aarch64-neon-intrinsics.c showing what happens if you pass, for example, a void* pointer to vld1_u32.



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10332
   case NEON::BI__builtin_neon_vld1q_v: {
+    auto PtrOp0 = EmitPointerWithAlignment(E->getArg(0));
     Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));
----------------
You can't call EmitPointerWithAlignment() here; we've already emitted the expression.  See CodeGenFunction::EmitARMBuiltinExpr for how we handle this on 32-bit ARM.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10334
     Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));
-    auto Alignment = CharUnits::fromQuantity(
-        BuiltinID == NEON::BI__builtin_neon_vld1_v ? 8 : 16);
-    return Builder.CreateAlignedLoad(VTy, Ops[0], Alignment);
+    return Builder.CreateAlignedLoad(VTy, Ops[0], PtrOp0.getAlignment());
   }
----------------
Might as well just `return Builder.CreateLoad(VTy, PtrOp0);`


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10363
   }
   case NEON::BI__builtin_neon_vst1_lane_v:
   case NEON::BI__builtin_neon_vst1q_lane_v:
----------------
Should we fix vst1 while we're in the area?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721