Pedro:

> -----邮件原件-----
> 发件人: [email protected] <[email protected]> 代表 Pedro Falcato
> 发送时间: 2023年5月15日 23:15
> 收件人: [email protected]
> 抄送: Pedro Falcato <[email protected]>; Michael D Kinney
> <[email protected]>; Liming Gao <[email protected]>;
> Zhiguang Liu <[email protected]>; Marvin Häuser
> <[email protected]>
> 主题: [edk2-devel] [PATCH v2 1/1] MdePkg/Base.h: Simply alignment
> expressions
> 
> Simplify ALIGN_VALUE and ALIGN_VALUE_ADDEND into simpler expressions.
> 
> ALIGN_VALUE can simply be a (value + (align - 1)) & ~align
> expression, which works for any power of 2 alignment and generates
> smaller code sequences. For instance:
>       ALIGN_VALUE(15, 16) = (15 + 15) & ~16 = 16
>       ALIGN_VALUE(16, 16) = (16 + 15) & ~16 = 16
> 
> Old codegen:
>       movq    %rdi, %rax
>       negq    %rax
>       andl    $15, %eax
>       addq    %rdi, %rax
> 
> New codegen:
>       leaq    15(%rdi), %rax
>       andq    $-16, %rax
> 
> ALIGN_VALUE_ADDEND can simply use a bitwise NOT of Value to get the
> addend for alignment, as, for instance:
>       ~15 & (16 - 1) = 1
>       15 + 1 = 16
> 

>       ~15 & (16 - 1) = 1
Its value should be zero, not 1. I also verify the updated ALIGN_VALUE_ADDEND. 
Its value is incorrect. Please double check. 

Thanks
Liming

> This change does not necessarily affect the end result, as the GCC and
> clang compilers were already able to see through things and optimize
> them into optimal instruction sequences, in the ALIGN_VALUE_ADDEND case.
> 
> Cc: Michael D Kinney <[email protected]>
> Cc: Liming Gao <[email protected]>
> Cc: Zhiguang Liu <[email protected]>
> Cc: Marvin Häuser <[email protected]>
> Signed-off-by: Pedro Falcato <[email protected]>
> ---
> 
> v2:
>       Addressed concerns expressed on Discord by Marvin
>               - Added missing parens around Alignment in ALIGN_VALUE
>               - Replaced -1 with -1U, as in the other macros.
> 
>  MdePkg/Include/Base.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> index 6597e441a6e2..422f80aff53d 100644
> --- a/MdePkg/Include/Base.h
> +++ b/MdePkg/Include/Base.h
> @@ -931,7 +931,7 @@ STATIC_ASSERT (ALIGNOF
> (__VERIFY_INT32_ENUM_SIZE) == sizeof (__VERIFY_INT32_ENUM
> 
>    @return  Addend to round Value up to alignment boundary Alignment.
>  **/
> -#define ALIGN_VALUE_ADDEND(Value, Alignment)  (((Alignment) - (Value))
> & ((Alignment) - 1U))
> +#define ALIGN_VALUE_ADDEND(Value, Alignment)  ((~(Value)) &
> ((Alignment) - 1U))
> 
>  /**
>    Rounds a value up to the next boundary using a specified alignment.
> @@ -945,7 +945,7 @@ STATIC_ASSERT (ALIGNOF
> (__VERIFY_INT32_ENUM_SIZE) == sizeof (__VERIFY_INT32_ENUM
>    @return  A value up to the next boundary.
> 
>  **/
> -#define ALIGN_VALUE(Value, Alignment)  ((Value) +
> ALIGN_VALUE_ADDEND (Value, Alignment))
> +#define ALIGN_VALUE(Value, Alignment)  (((Value) + ((Alignment) - 1U)) &
> ~(Alignment))
> 
>  /**
>    Adjust a pointer by adding the minimum offset required for it to be aligned
> on
> --
> 2.40.1
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104907): https://edk2.groups.io/g/devel/message/104907
Mute This Topic: https://groups.io/mt/98918460/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to