On 9 May 2016 at 18:22, Leif Lindholm <[email protected]> wrote: > On Tue, Apr 19, 2016 at 04:55:31PM +0200, Ard Biesheuvel wrote: >> We manage to use both an AND operation with 'gCacheAlignment - 1' and a >> modulo operation with 'gCacheAlignment' in the same compound if statement. >> Since gCacheAlignment is a global of which the compiler cannot guarantee >> that it is a power of two, > > Are you saying this is an "undefined behaviour" disaster waiting to > happen? >
No, not at all. It is just unnecessary to invoke the division routines. >> simply use the AND version, and use it against >> the bitwise OR of the two values to be tested. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <[email protected]> >> --- >> ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c >> b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c >> index 1e6b288b10b9..834afdba10ef 100644 >> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c >> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c >> @@ -92,8 +92,7 @@ DmaMap ( >> >> *Mapping = Map; >> >> - if ((((UINTN)HostAddress & (gCacheAlignment - 1)) != 0) || >> - ((*NumberOfBytes % gCacheAlignment) != 0)) { >> + if ((((UINTN)HostAddress | *NumberOfBytes) & (gCacheAlignment - 1)) != 0) >> { > > While the above is correct code, and less verbose, I am a bear of very > little brain, and OR:ing an address with a counter makes it hurt. > > Could the above instead be (more verbose but hopefully not relevantly > less efficient with a halfway decent compiler): > if ((((UINTN)HostAddress & (gCacheAlignment - 1)) != 0) || > (*NumberOfBytes & (gCacheAlignment - 1) != 0)) > > (It would have the benefit of making the diffstat immediately obvious.) > Yeah, that is also fine. I just noticed the mask and the modulo operation involving the same variable. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
