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

Lei Huang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 9 12:40:35 PDT 2020


lei 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]);
----------------
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.
```


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14289
+      else
+        ConstArg = (ConstArg * 4);
+      Ops[2] = ConstantInt::getSigned(Int32Ty, ConstArg);
----------------
```
ConstArg *= 4;
// Fix the constant according to endianess.
if (getTarget().isLittleEndian())
   ConstArg = 12 - ConstArg;
```


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14320
+        Call = Builder.CreateCall(F, Ops);
+    }
+    return Call;
----------------
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?


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

https://reviews.llvm.org/D83500





More information about the llvm-commits mailing list