Messages in this thread | | | From | Andrew Lutomirski <> | Date | Tue, 29 Mar 2011 07:52:52 -0400 | Subject | Re: [PATCH 2/6] x86-64: Don't generate cmov in vread_tsc |
| |
On Tue, Mar 29, 2011 at 2:15 AM, Ingo Molnar <mingo@elte.hu> wrote: > > * Andy Lutomirski <luto@MIT.EDU> wrote: > >> - /* This doesn't multiply 'zero' by anything, which *should* >> - * generate nicer code, except that gcc cleverly embeds the >> - * dereference into the cmp and the cmovae. Oh, well. >> + /* This doesn't multiply 'zero' by anything, which generates >> + * very slightly nicer code than multiplying it by 8. >> */ >> last = *( (cycle_t *) >> ((char *)&__vsyscall_gtod_data.clock.cycle_last + zero) ); >> >> - return ret >= last ? ret : last; >> + if (likely(ret >= last)) >> + return ret; >> + >> + /* GCC likes to generate cmov here, but this branch is extremely >> + predictable (it's just a funciton of time and the likely is >> + very likely) and there's a data dependence, so force GCC >> + to generate a branch instead. */ >> + asm volatile (""); > > barrier() would do the same, right?
Yes. It's overkill (the memory clobber is unnecessary) but should be harmless.
I'll take a look at folding this and [1/6] together and sticking them in a .S. The down side is that rdtsc_barrier() expends to two alternatives, and it has other callers. I'll see what it looks like.
> > Also, a nit, please use the customary (multi-line) comment style: > > /* > * Comment ..... > * ...... goes here. > */ > > specified in Documentation/CodingStyle.
I was hoping checkpatch would warn me :)
--Andy
> > Thanks, > > Ingo > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |