[PATCH] D143467: [PowerPC] Add target feature requirement to builtins

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.


================
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
----------------
qiucf wrote:
> 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.
After checking the source code, I found it might be too complex to filter existing features out. Because it actually has a syntax to support 'and'/'or' of feature requirements: https://github.com/llvm/llvm-project/blob/4016e5d/clang/lib/Basic/BuiltinTargetFeatures.h#L22_L31


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143467