On 10/13/2017 12:27 PM, Uros Bizjak wrote: > 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. Ah, you want to use it as a sequence within a peep2 match. That makes sense now.
OK for the trunk. jeff