"Andre Vieira (lists)" <andre.simoesdiasvie...@arm.com> writes:
> Apologies for the delay, had been waiting on some other relevant patches 
> to go in to make sure we didn't break any valid existing behaviours. It 
> should all be working properly now. I think I've also addressed all your 
> comments.  Most notable change is that it now uses the 'sorry' mechanism.
>
> Bootstrapped and regression tested on aarch64-none-linux-gnu.
>
>
> aarch64: remove SVE2 requirement from SME and diagnose it as unsupported
>
> As per the AArch64 ISA FEAT_SME does not require FEAT_SVE2, so we are 
> removing
> that false dependency in GCC.  However, we chose for now to not support this
> combination of features and will diagnose the combination of FEAT_SME 
> without
> FEAT_SVE2 as unsupported by GCC.  We may choose to support this in the 
> future.
>
> gcc/ChangeLog:
>
>       * config/aarch64/aarch64-arches.def (SME): Remove SVE2 as prerequisite
>       and add in FCMA and F16FML.
>       * config/aarch64/aarch64.cc (aarch64_override_options_internal):
>       Diagnose use of SME without SVE2.
>       * doc/invoke.texi (sme): Document that this can only be used with the
>       sve2 extension.
>
> gcc/testsuite/ChangeLog:
>
>       * gcc.target/aarch64/no-sve-with-sme-1.c: New.
>       * gcc.target/aarch64/no-sve-with-sme-2.c: New.
>       * gcc.target/aarch64/no-sve-with-sme-3.c: New.
>       * gcc.target/aarch64/sme/streaming_mode_4.c: Diagnose new error.
>       * gcc.target/aarch64/pragma_cpp_predefs_4.c: Pass +sve2 to existing
>       +sme pragma.
>       * gcc.target/aarch64/sve/acle/general-c/binary_int_opt_single_n_2.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binary_opt_single_n_2.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binary_single_1.c: Likewise.
>       * 
> gcc.target/aarch64/sve/acle/general-c/binary_za_slice_int_opt_single_1.c:
>       * gcc.target/aarch64/sve/acle/general-c/binary_za_slice_lane_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binary_za_slice_lane_2.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binary_za_slice_lane_3.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binary_za_slice_lane_4.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binary_za_slice_opt_single_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binary_za_slice_opt_single_2.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binary_za_slice_opt_single_3.c:
>       Likewise.
>       * 
> gcc.target/aarch64/sve/acle/general-c/binary_za_slice_uint_opt_single_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/binaryxn_2.c: Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/clamp_1.c: Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/compare_scalar_count_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/dot_za_slice_int_lane_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/dot_za_slice_lane_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/dot_za_slice_lane_2.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/dot_za_slice_uint_lane_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/shift_right_imm_narrowxn_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/storexn_1.c: Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/ternary_mfloat8_lane_1.c:
>       Likewise.
>       * 
> gcc.target/aarch64/sve/acle/general-c/ternary_mfloat8_lane_group_selection_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/ternary_qq_or_011_lane_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/unary_convertxn_1.c: Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/unary_convertxn_narrow_1.c:
>       Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/unaryxn_1.c: Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/write_za_1.c: Likewise.
>       * gcc.target/aarch64/sve/acle/general-c/write_za_slice_1.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlalb_lane_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlalb_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlallbb_lane_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlallbb_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlallbt_lane_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlallbt_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlalltb_lane_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlalltb_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlalltt_lane_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlalltt_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlalt_lane_mf8.c: Likewise.
>       * gcc.target/aarch64/sve2/acle/asm/mlalt_mf8.c: Likewise.

Thanks for the update.  I'll go through the tests properly later,
but it looks really good at first glance.  One thing I noticed though:

It would be nice if we didn't emit the sorry for:

> diff --git a/gcc/testsuite/gcc.target/aarch64/sme/streaming_mode_4.c 
> b/gcc/testsuite/gcc.target/aarch64/sme/streaming_mode_4.c
> index 
> 50e92f2e18a8f09829ce20d889a0fe7fe51bad7f..bc846c010a5ffb8d3886a840528db2e1a82ecfb4
>  100644
> --- a/gcc/testsuite/gcc.target/aarch64/sme/streaming_mode_4.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sme/streaming_mode_4.c
> @@ -1,4 +1,5 @@
>  // { dg-options "-mgeneral-regs-only" }
> +/* { dg-message "sorry, unimplemented: no support for 'sme' without 'sve2'" 
> "" { target *-*-* } 0 } */
>  
>  void sc_a () [[arm::streaming_compatible]] {}
>  void s_a () [[arm::streaming]] {} // { dg-error "streaming functions require 
> the ISA extension 'sme'" }

since the 'sme' in this case was added automatically by:

      if (isa_flags & AARCH64_FL_SM_ON)
        error ("streaming functions require the ISA extension %qs", "sme");
      else
        error ("functions with SME state require the ISA extension %qs",
               "sme");
      inform (input_location, "you can enable %qs using the command-line"
              " option %<-march%>, or by using the %<target%>"
              " attribute or pragma", "sme");
      opts->x_target_flags &= ~MASK_GENERAL_REGS_ONLY;
      auto new_flags = isa_flags | feature_deps::SME ().enable;
      aarch64_set_asm_isa_flags (opts, new_flags);

in order to avoid a string of later errors.  That's ok because the error
means that we won't generate code anyway.

So perhaps we should make that:

      auto new_flags = (isa_flags
                        | feature_deps::SME ().enable
                        | feature_deps::SVE2 ().enable);

instead.

Richard

Reply via email to