Re: RFC: Make builtin types only valid for some target features
On Sun, Dec 4, 2022 at 11:33 PM Richard Sandiford via Gcc
wrote:
>
> "Kewen.Lin" writes:
> > Hi,
> >
> > I'm working to find one solution for PR106736, which requires us to
> > make some built-in types only valid for some target features, and
> > emit error messages for the types when the condition isn't satisfied.
> > A straightforward idea is to guard the registry of built-in type under
> > the corresponding target feature. But as Peter pointed out in the
> > PR, it doesn't work, as these built-in types are used by some built-in
> > functions which are required to be initialized always regardless of
> > target features, in order to support target pragma/attribute. For
> > the validity checking on the built-in functions, it happens during
> > expanding the built-in functions calls, since till then we already
> > know the exact information on specific target feature.
> >
> > One idea is to support built-in type checking in a similar way, to
> > check if all used type_decl (built-in type) are valid or not somewhere.
> > I hacked to simply check currently expanding gimple stmt is gassign
> > and further check the types of its operands, it did work but checking
> > gassign isn't enough. Maybe I missed something, there seems not an
> > efficient way for a full check IMHO.
> >
> > So I tried another direction, which was inspired by the existing
> > attribute altivec, to introduce an artificial type attribute and the
> > corresponding macro definition, during attribute handling it can check
> > target option node about target feature for validity. The advantage
> > is that the checking happens in FE, so it reports error early, and it
> > doesn't need a later full checking on types. But with some prototyping
> > work, I found one issue that it doesn't support param decl well, since
> > the handling on attributes of function decl happens after that on
> > attributes of param decl, so we aren't able to get exact target feature
> > information when handling the attributes on param decl. It requires
> > front-end needs to change the parsing order, I guess it's not acceptable?
> > So I planed to give up and return to the previous direction.
> >
> > Does the former idea sound good? Any comments/suggestions, and other
> > ideas?
> >
> > Thanks a lot in advance!
>
> FWIW, the aarch64 fp move patterns emit the error directly. They then
> expand an integer-mode move, to provide some error recovery. (The
> alternative would be to make the error fatal.)
>
> (define_expand "mov"
> [(set (match_operand:GPF_TF_F16_MOV 0 "nonimmediate_operand")
> (match_operand:GPF_TF_F16_MOV 1 "general_operand"))]
> ""
> {
> if (!TARGET_FLOAT)
> {
> aarch64_err_no_fpadvsimd (mode);
> machine_mode intmode
> = int_mode_for_size (GET_MODE_BITSIZE (mode), 0).require ();
> emit_move_insn (gen_lowpart (intmode, operands[0]),
> gen_lowpart (intmode, operands[1]));
> DONE;
> }
>
> This isn't as user-friendly as catching the error directly in the FE,
> but I think in practice it's going to be very hard to trap all invalid
> uses of a type there. Also, the user error in these situations is likely
> to be forgetting to enable the right architecture feature, rather than
> accidentally using the wrong type. So an error about missing architecture
> features is probably good enough in most cases.
I did have a patch which improved the situation for the SVE types to
provide an error message at compile time when SVE is not enabled
but I didn't get any feedback from either the C or C++ front-end folks.
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583786.html
I suspect if that patch gets reviewed by the front-end folks, Kewen
could use the same infrastructure to error out on the types for rs6000
backend.
Thanks,
Andrew Pinski
>
> Thanks,
> Richard
Re: RFC: Make builtin types only valid for some target features
Hi Richard,
on 2022/12/5 15:31, Richard Sandiford wrote:
> "Kewen.Lin" writes:
>> Hi,
>>
>> I'm working to find one solution for PR106736, which requires us to
>> make some built-in types only valid for some target features, and
>> emit error messages for the types when the condition isn't satisfied.
>> A straightforward idea is to guard the registry of built-in type under
>> the corresponding target feature. But as Peter pointed out in the
>> PR, it doesn't work, as these built-in types are used by some built-in
>> functions which are required to be initialized always regardless of
>> target features, in order to support target pragma/attribute. For
>> the validity checking on the built-in functions, it happens during
>> expanding the built-in functions calls, since till then we already
>> know the exact information on specific target feature.
>>
>> One idea is to support built-in type checking in a similar way, to
>> check if all used type_decl (built-in type) are valid or not somewhere.
>> I hacked to simply check currently expanding gimple stmt is gassign
>> and further check the types of its operands, it did work but checking
>> gassign isn't enough. Maybe I missed something, there seems not an
>> efficient way for a full check IMHO.
>>
>> So I tried another direction, which was inspired by the existing
>> attribute altivec, to introduce an artificial type attribute and the
>> corresponding macro definition, during attribute handling it can check
>> target option node about target feature for validity. The advantage
>> is that the checking happens in FE, so it reports error early, and it
>> doesn't need a later full checking on types. But with some prototyping
>> work, I found one issue that it doesn't support param decl well, since
>> the handling on attributes of function decl happens after that on
>> attributes of param decl, so we aren't able to get exact target feature
>> information when handling the attributes on param decl. It requires
>> front-end needs to change the parsing order, I guess it's not acceptable?
>> So I planed to give up and return to the previous direction.
>>
>> Does the former idea sound good? Any comments/suggestions, and other
>> ideas?
>>
>> Thanks a lot in advance!
>
> FWIW, the aarch64 fp move patterns emit the error directly. They then
> expand an integer-mode move, to provide some error recovery. (The
> alternative would be to make the error fatal.)
>
> (define_expand "mov"
> [(set (match_operand:GPF_TF_F16_MOV 0 "nonimmediate_operand")
> (match_operand:GPF_TF_F16_MOV 1 "general_operand"))]
> ""
> {
> if (!TARGET_FLOAT)
> {
> aarch64_err_no_fpadvsimd (mode);
> machine_mode intmode
> = int_mode_for_size (GET_MODE_BITSIZE (mode), 0).require ();
> emit_move_insn (gen_lowpart (intmode, operands[0]),
> gen_lowpart (intmode, operands[1]));
> DONE;
> }
>
> This isn't as user-friendly as catching the error directly in the FE,
> but I think in practice it's going to be very hard to trap all invalid
> uses of a type there. Also, the user error in these situations is likely
> to be forgetting to enable the right architecture feature, rather than
> accidentally using the wrong type. So an error about missing architecture
> features is probably good enough in most cases.
>
Thanks a lot for your comments! Yes, it's a question if it's worth to
spending non-trivial efforts to check and report all invalid uses. For
the case in PR106736, it's enough to check in mov[ox]o define_expand
like the example you provided above, one trial patch was posted previously
at[1]. I think one difference is that we want to further check the actual
type instead of the mode, as Peter said "'types' are limited to MMA, but
the opaque modes are not limited to MMA.".
[1] https://gcc.gnu.org/bugzilla/attachment.cgi?id=53522&action=diff
BR,
Kewen
Re: access to include path in front end
Hey,
On Fri, 2 Dec 2022, James K. Lowden wrote:
> > > 3. Correct the entries in the default_compilers array. Currently I
> > > have in cobol/lang-specs.h:
> > >
> > > {".cob", "@cobol", 0, 0, 0},
> > > {".COB", "@cobol", 0, 0, 0},
> > > {".cbl", "@cobol", 0, 0, 0},
> > > {".CBL", "@cobol", 0, 0, 0},
> > > {"@cobol",
> > > "cobol1 %i %(cc1_options) %{!fsyntax-only:%(invoke_as)}",
> > > 0, 0, 0},
> >
> > It misses %(cpp_unique_options) which was the reason why your -I
> > arguments weren't passed to cobol1.
>
> If I understood you correctly, I don't need to modify gcc.cc. I only
> need to modify cobol/lang-specs.h, which I've done. But that's
> evidently not all I need to do, because it doesn't seem to work.
>
> The last element in the fragment in cobol/lang-specs.h is now:
>
> {"@cobol",
> "cobol1 %i %(cc1_options) %{!fsyntax-only:%(invoke_as)} "
> "%(cpp_unique_options) ",
%(invoke_as) needs to be last. What it does is effectively add this to
the command line (under certain conditions): "-somemoreoptions | as".
Note the pipe symbol. Like in normal shell commands also the gcc driver
interprets this as "and now start the following command as well connection
stdout of the first to stdin of the second". So all in all the generated
cmdline will be somewhat like:
cobol1 input.cbl -stuff-from-cc1-options | as - -stuff-from-cpp-options
Your cpp_unique_options addition will effectively be options to that 'as'
command, but you wanted it to be options for cobol1. So, just switch
order of elements.
> I see the -B and -I options, and others, with their arguments, contained
> in COLLECT_GCC_OPTIONS on lines 9 and 11. I guess that represents an
> environment string?
Yes. It's our round-about-way of passing the gcc options as the user gave
them downwards in case collect2 (a wrapper for the linking step for, gah,
don't ask) needs to call gcc itself recursively. But in the -### (or -v)
output, if the assembler is invoked in your example (i.e. cobol1 doesn't
fail for some reason) you should see your -I options being passed to that
one (wrongly so, of course :) ).
Ciao,
Michael.
Re: RFC: Make builtin types only valid for some target features
On Mon, Dec 05, 2022 at 07:31:48AM +, Richard Sandiford wrote:
> FWIW, the aarch64 fp move patterns emit the error directly. They then
> expand an integer-mode move, to provide some error recovery. (The
> alternative would be to make the error fatal.)
>
> (define_expand "mov"
> [(set (match_operand:GPF_TF_F16_MOV 0 "nonimmediate_operand")
> (match_operand:GPF_TF_F16_MOV 1 "general_operand"))]
> ""
> {
> if (!TARGET_FLOAT)
> {
> aarch64_err_no_fpadvsimd (mode);
> machine_mode intmode
> = int_mode_for_size (GET_MODE_BITSIZE (mode), 0).require ();
> emit_move_insn (gen_lowpart (intmode, operands[0]),
> gen_lowpart (intmode, operands[1]));
> DONE;
> }
>
> This isn't as user-friendly as catching the error directly in the FE,
> but I think in practice it's going to be very hard to trap all invalid
> uses of a type there. Also, the user error in these situations is likely
> to be forgetting to enable the right architecture feature, rather than
> accidentally using the wrong type. So an error about missing architecture
> features is probably good enough in most cases.
The obvious problwem with this is you have to write it separately for
each and every such pattern. But of course the opaque modes only ever
have mov patterns, everything else goes via builtins, so if this is the
best we can easily do, so be it :-)
Thanks,
Segher
Re: RFC: Make builtin types only valid for some target features
Hi Andrew,
on 2022/12/5 18:10, Andrew Pinski wrote:
> On Sun, Dec 4, 2022 at 11:33 PM Richard Sandiford via Gcc
> wrote:
>>
>> "Kewen.Lin" writes:
>>> Hi,
>>>
>>> I'm working to find one solution for PR106736, which requires us to
>>> make some built-in types only valid for some target features, and
>>> emit error messages for the types when the condition isn't satisfied.
>>> A straightforward idea is to guard the registry of built-in type under
>>> the corresponding target feature. But as Peter pointed out in the
>>> PR, it doesn't work, as these built-in types are used by some built-in
>>> functions which are required to be initialized always regardless of
>>> target features, in order to support target pragma/attribute. For
>>> the validity checking on the built-in functions, it happens during
>>> expanding the built-in functions calls, since till then we already
>>> know the exact information on specific target feature.
>>>
>>> One idea is to support built-in type checking in a similar way, to
>>> check if all used type_decl (built-in type) are valid or not somewhere.
>>> I hacked to simply check currently expanding gimple stmt is gassign
>>> and further check the types of its operands, it did work but checking
>>> gassign isn't enough. Maybe I missed something, there seems not an
>>> efficient way for a full check IMHO.
>>>
>>> So I tried another direction, which was inspired by the existing
>>> attribute altivec, to introduce an artificial type attribute and the
>>> corresponding macro definition, during attribute handling it can check
>>> target option node about target feature for validity. The advantage
>>> is that the checking happens in FE, so it reports error early, and it
>>> doesn't need a later full checking on types. But with some prototyping
>>> work, I found one issue that it doesn't support param decl well, since
>>> the handling on attributes of function decl happens after that on
>>> attributes of param decl, so we aren't able to get exact target feature
>>> information when handling the attributes on param decl. It requires
>>> front-end needs to change the parsing order, I guess it's not acceptable?
>>> So I planed to give up and return to the previous direction.
>>>
>>> Does the former idea sound good? Any comments/suggestions, and other
>>> ideas?
>>>
>>> Thanks a lot in advance!
>>
>> FWIW, the aarch64 fp move patterns emit the error directly. They then
>> expand an integer-mode move, to provide some error recovery. (The
>> alternative would be to make the error fatal.)
>>
>> (define_expand "mov"
>> [(set (match_operand:GPF_TF_F16_MOV 0 "nonimmediate_operand")
>> (match_operand:GPF_TF_F16_MOV 1 "general_operand"))]
>> ""
>> {
>> if (!TARGET_FLOAT)
>> {
>> aarch64_err_no_fpadvsimd (mode);
>> machine_mode intmode
>> = int_mode_for_size (GET_MODE_BITSIZE (mode), 0).require ();
>> emit_move_insn (gen_lowpart (intmode, operands[0]),
>> gen_lowpart (intmode, operands[1]));
>> DONE;
>> }
>>
>> This isn't as user-friendly as catching the error directly in the FE,
>> but I think in practice it's going to be very hard to trap all invalid
>> uses of a type there. Also, the user error in these situations is likely
>> to be forgetting to enable the right architecture feature, rather than
>> accidentally using the wrong type. So an error about missing architecture
>> features is probably good enough in most cases.
>
> I did have a patch which improved the situation for the SVE types to
> provide an error message at compile time when SVE is not enabled
> but I didn't get any feedback from either the C or C++ front-end folks.
> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583786.html
>
Nice! Many thanks for providing this new direction.
> I suspect if that patch gets reviewed by the front-end folks, Kewen
> could use the same infrastructure to error out on the types for rs6000
> backend.
Yeah, I just confirmed that on top of your patch introducing function
rs6000_verify_type_context to take care of those MMA types can fix the
issue in PR106736. TBH, I'm not sure if your patch can cover all
possible places where a built-in type can be used, but I guess it can
cover the most.
BR,
Kewen
Re: [PATCH] Various pages: SYNOPSIS: Use VLA syntax in function parameters
On Sat, 3 Dec 2022, Alejandro Colomar via Gcc wrote: > What do you think about it? I'm not asking for your opinion about adding it > to GCC, but rather for replacing the current '.' in the man-pages before I > release later this month. Do you think I should apply that change? I think man pages should not use any novel syntax - even syntax newly added to the C standard or GCC, unless required to express the standard prototype for a function. They should be written for maximal comprehensibility to C users in general, who are often behind on knowledge standard features let alone the more obscure extensions - and certainly don't know about random, highly speculative suggestions for possible features suggested in random mailing list threads. So: don't use any invented syntax (even if you explain it somewhere in the man pages), don't use any syntax newly introduced in C23 unless strictly necessary and you're sure it's already extremely widely understood among C users, be wary of syntax introduced in C11. If a new feature in this area were introduced in C29, waiting at least several years after that standard is released (*not* just after the feature gets added to a draft) to start using the new syntax in man pages would be a good idea. -- Joseph S. Myers jos...@codesourcery.com
Using [[may_alias]] in C23/C++23 on a union works in neither post-"union" position, or at the end of the definition
The reproduction is below. Not sure if this is intended or a bug, sorry to
clutter up the mailing list if it's intended!
union __attribute__((may_alias)) works {};
// :3:18: note: attribute for 'union broken2' must follow the
'union' keyword
union broken1 {} [[may_alias]];
// Okay, so let's move it so it follows union...
// :7:21: warning: 'may_alias' attribute directive ignored
[-Wattributes]
// 7 | union [[may_alias]] broken2 {};
//| ^~~
union [[may_alias]] broken2 {};
