On 08/07/2019 21:28, Julien Grall wrote: >>>> >>>> https://lore.kernel.org/xen-devel/1561109798-8744-5-git-send-email-andrew.coop...@citrix.com/T/#t >>>> >>>> >>> >>> To be honest, I think it is wrong to try to micro-optimize a common >>> prototype for the benefit of one architecture and one compiler version >>> (or even none per the e-mail). >> >> The prototype wasn't common. Observe that before this patch, ARM used >> unsigned long while x86 used uint16_t. It should become common, >> however. > > I am not sure to follow this. AFAICT, we use uint16_t properly on Arm.
Look at the modifications to gnttab_clear_flags(), and in particular, the removals. Before this patch, the API really is different between ARM and x86. (Although it differed between unsigned long and unsigned int. The uint16_t was a mistake on my behalf.) > >> >> In practice, we're talking about bits 3 and 4, and this isn't liable to >> change in a hurry. >> >>> One could also argue that this may be not beneficial for the non-x86 >>> architecture depending on how the compiler decide to do the cast from >>> 32-bit to 16-bit... >> >> All architecture necessarily suffer the downcast somewhere, even x86. >> ARM's is in the prototype for guest_clear_mask16(), but in terms of the >> common logic for calculating conditionally which bits to clear, keeping >> everything as unsigned int for as long as possible offers the most >> flexibility to the compiler, as it can see all the constants involved. > > This is the argument I was looking for :). Thank you for writing it! Can I take this as an ack, or a request for clarification in the commit message, or something else? (This detail is the final outstanding piece for the series to be committed.) ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
