From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751391AbdBXTIB (ORCPT ); Fri, 24 Feb 2017 14:08:01 -0500 Received: from mga07.intel.com ([134.134.136.100]:49850 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751275AbdBXTHm (ORCPT ); Fri, 24 Feb 2017 14:07:42 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,201,1484035200"; d="scan'208";a="937798857" Message-ID: <1487963247.115017.10.camel@ranerica-desktop> Subject: Re: [PATCH v4 01/17] x86/mpx: Do not use SIB index if index points to R/ESP From: Ricardo Neri To: Nathan Howard Cc: Joe Perches , Thomas Gleixner , Ingo Molnar , linux-msdos@vger.kernel.org, Lorenzo Stoakes , "Ravi V. Shankar" , Andrew Morton , "Michael S. Tsirkin" , Adam Buchbinder , Alexandre Julliard , Qiaowei Ren , Vlastimil Babka , Chris Metcalf , wine-devel@winehq.org, Jiri Slaby , x86@kernel.org, Liang Z Li , Stas Sergeev , Paolo Bonzini , Dave Hansen , Paul Gortmaker , Masami Hiramatsu , Jonathan Corbet , Andy Lutomirski , Brian Gerst , Colin Ian King , Shuah Khan , Fenghua Yu , linux-kernel@vger.kernel.org, Borislav Petkov , Chen Yucong , Huang Rui , "H. Peter Anvin" , Peter Zijlstra Date: Fri, 24 Feb 2017 11:07:27 -0800 In-Reply-To: References: <20170223063706.71554-1-ricardo.neri-calderon@linux.intel.com> <20170223063706.71554-2-ricardo.neri-calderon@linux.intel.com> <20170223072432.GX6515@twins.programming.kicks-ass.net> <1487888232.115017.4.camel@ranerica-desktop> <1487903602.14159.50.camel@perches.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2017-02-24 at 09:47 -0500, Nathan Howard wrote: > Also, this code would read better with the inner test > reversed or done first > > if (indx_offset < 0) { > if (indx_offset != -EDOM) > goto out_err; > indx = 0; > } else { > indx = regs_get_register(etc...) > } > > or > if (indx_offset == -EDOM) > indx = 0; > else if (indx_offset < 0) > goto err; > > > Or goto out_err; > > > else > indx = regs_get_register(etc...) > > The compiler should generate the same code in any > case, but either could improve reader understanding. > > > Also, it may be a tweak more efficient to handle the most likely > runtime case in the conditional stack first (whichever that may be). The most likely case will be a positive value but I need to check for negatives first :( I could wrap the first conditional in an "unlikely". Thanks and BR, Ricardo