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