[PATCH] D83500: [PowerPC][Power10] Implement custom codegen for the vec_replace_elt and vec_replace_unaligned builtins.

[Amy Kwan via Phabricator via cfe-commits]

amyk marked 3 inline comments as done.
amyk added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14273
+    // The third argument to vec_replace_elt will be emitted to either
+    // the vinsw or vinsd instruction. It must be a compile time constant.
+    ConstantInt *ArgCI = dyn_cast<ConstantInt>(Ops[2]);
----------------
lei wrote:
> Do you mean?
> ```
>     // The third argument of vec_replace_elt must be a compile time constant and will be emitted either
>     //  to the vinsw or vinsd instruction.
> ```
Yes. Thank you - I will update the wording here and in the other builtin.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14320
+        Call = Builder.CreateCall(F, Ops);
+    }
+    return Call;
----------------
lei wrote:
> What are the chances of reaching to the end of this if/else-if section and `Call` is null? ie `getPrimitiveSizeInBits() != [32|64]`
> I feel like it would be better if we can structure it so that we are not doing all these nesting of `if`s and just do returns within the diff if-conditions.
> 
> Have you tried to pull out the diff handling of 32/64bit arg and consolidating the code a bit?
Thanks - I realize that I should probably pull the `Call` out. I'll update this. 
I've actually consolidated the code quite a bit already, but I'll see if I can make any further improvements on this.


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

https://reviews.llvm.org/D83500