On Wed, Nov 13, 2013 at 12:27:10PM +1000, Richard Henderson wrote: > On 11/13/2013 08:53 AM, Paolo Bonzini wrote: > > Il 12/11/2013 19:54, Richard Henderson ha scritto: > >> For what it's worth, I think BOTH of the patches that have been posted > >> should be applied. That is, the patch that does (X || 1) -> (1 || X), > >> and the patch that adds the stub. > >> > >> Frankly I'd have thought this was obvious > > > > It's not that obvious to me. > > > > If you add the stub, the patch that reorders operands is not necessary. > > If you reorder operands, the stub is not necessary. > > > > The patch that does (X || 1) -> (1 || X) is unnecessary as a > > microoptimization, since this code basically runs once at startup. The > > code is also a little bit less clear with the reordered operands, but > > perhaps that's just me because I wrote the code that way. (Splitting > > the if in two would also make sense, and would not affect clarity). > > > > Why should both be applied? > > It's worth working around the clang missed optimization, if for nothing else > than avoiding the noise of the bugs that would otherwise be filed against the > release. > > I think it's also worthwhile to implement the kvm api in kvm-stub.c, > unnecessary or not. If you really want compile-time feedback on those that > ought to have been removed by optimization, you could elide them from the stub > file depending on ifndef __OPTIMIZE__. > Sounds like a nice compromise. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
