This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/3] optabs: ensure mem_thread_fence is a compiler barrier


Alexander Monakov <amonakov@ispras.ru> writes:
> diff --git a/gcc/optabs.c b/gcc/optabs.c
> index a9900657a58..d568ca376fe 100644
> --- a/gcc/optabs.c
> +++ b/gcc/optabs.c
> @@ -6298,7 +6298,12 @@ void
>  expand_mem_thread_fence (enum memmodel model)
>  {
>    if (targetm.have_mem_thread_fence ())
> -    emit_insn (targetm.gen_mem_thread_fence (GEN_INT (model)));
> +    {
> +      rtx_insn *last = get_last_insn ();
> +      emit_insn (targetm.gen_mem_thread_fence (GEN_INT (model)));
> +      if (last == get_last_insn () && !is_mm_relaxed (model))
> +	expand_asm_memory_barrier ();
> +    }
>    else if (!is_mm_relaxed (model))
>      {
>        if (targetm.have_memory_barrier ())

It would be simpler to test whether targetm.gen_mem_thread_fence
returns NULL.

This feels a bit hacky though.  Checking whether a generator produced no
instructions is usually the test for whether the generator FAILed, which
should normally be treated as though the pattern wasn't defined to start
with.  This is useful if the FAIL condition is too complex to put in the
define_expand/insn C condition.

In those contexts, if a generator wants to succeed and generate no
instructions, it should emit a note instead.  (Yes, it would be good
to have a nicer interface...)

So IMO it's confusing to overload the empty sequence to mean something
else in this context, and it wouldn't work if a target does emit the kind
of "I didn't FAIL" note just mentioned.

FWIW, I agree with Jeff in the previous thread that (compared to this)
it would be more robust to make optabs.c emit the compile barrier
unconditionally and just leave the target to emit a machine barrier.

Your earlier patch to make targets emit at least a compile barrier
seemed good too.  Perhaps that way we can treat an empty sequence as a
FAIL in the normal way, or assert that the pattern doesn't FAIL.

Thanks,
Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]