qiucf added inline comments.

================
Comment at: clang/include/clang/Basic/BuiltinsPPC.def:444
+TARGET_BUILTIN(__builtin_altivec_vcmpnew_p, "iiV4iV4i", "", "power9-vector")
+TARGET_BUILTIN(__builtin_altivec_vcmpned_p, "iiV2LLiV2LLi", "", "altivec")
+
----------------
maryammo wrote:
> amyk wrote:
> > Does this need to be `vsx`?
> How do we find the appropriate FEATURE for the above 4 builtins?  (first 3 
> are p9 and the 4th one is altivec)
`vcmpneb` `vcmpneh` `vcmpnew` debute in ISA 3.0. `vcmpned` does not exist, so 
it's keeped as-is. (but `vector long long` requires vsx, so it's reasonable to 
require vsx)


================
Comment at: clang/include/clang/Basic/BuiltinsPPC.def:987
+
+UNALIASED_CUSTOM_BUILTIN(mma_assemble_acc, "vW512*VVVV", false, "mma")
+UNALIASED_CUSTOM_BUILTIN(mma_disassemble_acc, "vv*W512*", false, "mma")
----------------
kamaub wrote:
> stefanp wrote:
> > Based on the original implementation in `SemaBuiltinPPCMMACall` all of the 
> > `mma` builtins also require `paired-vector-memops`. 
> > Is this something that we still need?
> since we are able to supply a comma separated list as done with 
> `TARGET_BUILTIN(__builtin_ppc_compare_exp_uo, "idd", "", 
> "isa-v30-instructions,vsx")` @ 
> `clang/include/clang/Basic/BuiltinsPPC.def:105`we should definitely also 
> specify `paired-vector-memops,mma` for the `[UNALIASED_]CUSTOM_BUILTIN`s 
> previously covered under the default case of `SemaBuiltinPPCMMACall()` 
Since `mma` and `paired-vector-memops` are independent from each other, I think 
only assemble/disassemble builtins should require `paired-vector-memops`?


================
Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-test.c:47
 
-int test_test_data_class_f() {
-// CHECK-LABEL:       @test_test_data_class_f
-// CHECK:             [[TMP:%.*]] = call i32 
@llvm.ppc.test.data.class.f32(float %0, i32 127)
-// CHECK-NEXT:        ret i32 [[TMP]]
-// CHECK-NONPWR9-ERR: error: this builtin is only valid on POWER9 or later CPUs
-// CHECK-NOVSX-ERR: error: this builtin requires VSX to be enabled
-  return __test_data_class(f, 127);
+// CHECK-NOVSX-ERR: error: '__builtin_ppc_compare_exp_uo' needs target feature 
isa-v30-instructions,vsx
+// CHECK-NOVSX-ERR: error: '__builtin_ppc_compare_exp_lt' needs target feature 
isa-v30-instructions,vsx
----------------
stefanp wrote:
> nit:
> Should this be 
> ```
> ... needs target feature vsx
> ```
> Instead of listing them both?
> 
> Fixing this might be more trouble than it's worth because you would have to 
> edit `CodeGenFunction::checkTargetFeatures`. I just thought I would mention 
> it.
Yes. That can be done in another patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143467

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to