On Wed, Jun 28, 2023 at 12:32 AM Julian Waters <tanksherma...@gmail.com> wrote:
>
> Hi Andrew,
>
> On a Microsoft Windows target the following (placed inside a function of
> course) will only work correctly if volatile is specified in the basic asm
> block (or if the attached patch was applied to gcc):
These inline-asm will never work correctly .... even with volatile
because you change the sp behind the back of GCC and the control flow
too.
Also I suspect you want gnu::noipa rather than gnu::noinline for the
lambda there as I suspect the IPA passes are getting rid of the
function call thinking it is just pure/const.
Rather than related to the inline-asm being volatile or not.
Thanks,
Andrew
>
> asm ("1:" "\n"
> "\t" ".seh_handler __C_specific_handler, @except" "\n"
> "\t" ".seh_handlerdata" "\n"
> "\t" ".long 1" "\n"
> "\t" ".rva 1b, 2f, 3f, 4f" "\n"
> "\t" ".seh_code");
>
> {
> // std::printf("Guarded\n");
> RaiseException(EXCEPTION_BREAKPOINT, 0, 0, nullptr);
> }
>
> asm ("nop" "\n"
> "\t" "2: nop" "\n"
> "\t" "jmp 5f" "\n"
> "\t" "3:" "\n"
> "\t" "push rbp" "\n"
> "\t" "mov rbp, rsp"
> "\t" "push r15" "\n"
> "\t" "mov r15, rcx" "\n");
>
> [] [[gnu::noinline, gnu::used]] () -> long {
> return EXCEPTION_EXECUTE_HANDLER;
> }();
>
> asm ("pop r15" "\n"
> "\t" "pop rbp" "\n"
> "\t" "ret" "\n"
> "\t" "nop" "\n"
> "4:");
>
> {
> std::printf("Exception\n");
> }
>
> asm ("5:");
>
> In any case I doubt marking it as volatile in the parser hurts either, since
> this is the behaviour it's supposed to have
>
> best regards,
> Julian
>
> On Wed, Jun 28, 2023 at 12:24 AM Andrew Pinski <pins...@gmail.com> wrote:
>>
>> On Tue, Jun 27, 2023 at 9:15 AM Julian Waters <tanksherma...@gmail.com>
>> wrote:
>> >
>> > Hi Andrew,
>> >
>> > That can't be right, on my system a test of asm vs asm volatile with -O3
>> > and -flto=auto yields very different results, with only the latter being
>> > correct. The patch fixed it and caused gcc to emit correct assembly
>>
>> Can you provide a few testcases? Because the gimplifier should always happen.
>>
>> Thanks,
>> Andrew Pinski
>>
>> >
>> > best regards,
>> > Julian
>> >
>> > On Wed, Jun 28, 2023 at 12:08 AM Andrew Pinski <pins...@gmail.com> wrote:
>> >>
>> >> On Tue, Jun 27, 2023 at 9:03 AM Julian Waters via Gcc <gcc@gcc.gnu.org>
>> >> wrote:
>> >> >
>> >> > gcc's documentatation mentions that all basic asm blocks are always
>> >> > volatile,
>> >> > yet the parser fails to account for this by only ever setting
>> >> > volatile_p to true
>> >> > if the volatile qualifier is found. This patch fixes this by adding a
>> >> > special case check for extended_p before finish_asm_statement is called
>> >>
>> >> The patch which are you doing will not change the behavior of GCC as
>> >> GCC already treats them as volatile later on.
>> >> non-extended inline-asm has no outputs so the following code in the
>> >> gimplifier will kick in and turn the gimple statement into volatile:
>> >> gimple_asm_set_volatile (stmt, ASM_VOLATILE_P (expr) || noutputs ==
>> >> 0);
>> >>
>> >> (note I am about to push a patch which changes the condition slightly
>> >> to have `asm goto` as volatile).
>> >>
>> >> Thanks,
>> >> Andrew
>> >>
>> >> >
>> >> > From 3094be39e3e65a6a638f05fafd858b89fefde6b5 Mon Sep 17 00:00:00 2001
>> >> > From: TheShermanTanker <tanksherma...@gmail.com>
>> >> > Date: Tue, 27 Jun 2023 23:56:38 +0800
>> >> > Subject: [PATCH] asm not using extended syntax should always be volatile
>> >> >
>> >> > ---
>> >> > gcc/cp/parser.cc | 3 +++
>> >> > 1 file changed, 3 insertions(+)
>> >> >
>> >> > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
>> >> > index a6341b9..ef3d06a 100644
>> >> > --- a/gcc/cp/parser.cc
>> >> > +++ b/gcc/cp/parser.cc
>> >> > @@ -22355,6 +22355,9 @@ cp_parser_asm_definition (cp_parser* parser)
>> >> > /* Create the ASM_EXPR. */
>> >> > if (parser->in_function_body)
>> >> > {
>> >> > + if (!extended_p) {
>> >> > + volatile_p = true;
>> >> > + }
>> >> > asm_stmt = finish_asm_stmt (asm_loc, volatile_p, string, outputs,
>> >> > inputs, clobbers, labels, inline_p);
>> >> > /* If the extended syntax was not used, mark the ASM_EXPR. */
>> >> > --
>> >> > 2.35.1.windows.2
