[PATCH] D46273: [InstCombine, ARM] Convert vld1 to llvm load

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:935
+                                 InstCombiner::BuilderTy &Builder) {
+  auto IntrAlign = dyn_cast<ConstantInt>(II.getArgOperand(1));
+  unsigned Alignment = IntrAlign && IntrAlign->getZExtValue() < MemAlign ?
----------------
javed.absar wrote:
> There is a check of IntrAlign, but then IntrAlign is still used otherwise? Maybe it will be cleaner to not use 'auto'
Using auto as it's inferred by the dynamic cast.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:2987
+  case Intrinsic::arm_neon_vld1: {
+    unsigned MemAlign = getKnownAlignment(II->getArgOperand(0),
+                                          DL, II, &AC, &DT);
----------------
javed.absar wrote:
> It might be cleaner to extract MemAlign in the called function
I was thinking that I'll have to pass three pointers as parameters to make that happen, which doesn't sound very optimal performance wise.


https://reviews.llvm.org/D46273