On Fri, Oct 13, 2017 at 7:30 PM, Jeff Law <l...@redhat.com> wrote: > On 09/05/2017 07:50 AM, Uros Bizjak wrote: >> Revised patch, incorporates fixes from Alexander's review comments. >> >> I removed some implementation details from Alexander's description of >> memory_blockage named pattern. >> >> >> 2017-09-05 Uros Bizjak <ubiz...@gmail.com> >> >> * target-insns.def: Add memory_blockage. >> * optabs.c (expand_memory_blockage): New function. >> (expand_asm_memory_barrier): Rename ... >> (expand_asm_memory_blockage): ... to this. >> (expand_mem_thread_fence): Call expand_memory_blockage >> instead of expand_asm_memory_barrier. >> (expand_mem_singnal_fence): Ditto. >> (expand_atomic_load): Ditto. >> (expand_atomic_store): Ditto. >> * doc/md.texi (Standard Pattern Names For Generation): >> Document memory_blockage instruction pattern. >> >> Bootstrapped and regression tested together with a followup x86 patch >> on x86_64-linux-gnu {,-m32}. >> >> OK for mainline? > SO I don't see anything technically wrong here. But what I don't > understand is the value in providing another way to do the same thing. > > I see a patch that introduces calls to gen_memory_blockage in the x86 > backend. But what I don't see is why those couldn't just be > expand_asm_memory_barrier? > > Were you planning a x86 specific expander? If so what advantages does > it have over the asm-style variant? > > I feel like I'm missing something here.
Yes: Alexander's patch generates asm-style blockage instruction in the generic expansion part. If we want to include this instruction in the peephole2 sequence, we need a different (simple and deterministic) instruction with known (simple) RTL pattern. Proposed middle end patch allows us to emit target-dependant UNSPEC pattern instead of a generic asm-style pattern for the blockage instruction. This way, we can include target-dependent UNSPEC in peephole2 sequence, as shown in a follow-up target patch. Uros.