public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Pankaj Bansal <pankaj.bansal@nxp.com>
To: Laszlo Ersek <lersek@redhat.com>,
	"Gao, Liming" <liming.gao@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [RFC] MdePkg/BaseLib: Change BitField functions.
Date: Mon, 26 Feb 2018 06:33:57 +0000	[thread overview]
Message-ID: <AM0PR0402MB3940128A7CB425D548CC1B04F1C10@AM0PR0402MB3940.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <48d7d1a9-503f-2a72-803d-f63a8bc67e9c@redhat.com>

Hello Laszlo,

Thanks for your explanation.
Now I understand the usage of bit field functions.

Regards,
Pankaj Bansal

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, February 23, 2018 5:43 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>; Gao, Liming
> <liming.gao@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2] [RFC] MdePkg/BaseLib: Change BitField functions.
> 
> On 02/23/18 12:16, Pankaj Bansal wrote:
> > Hi Liming Gao,
> >
> > Thanks for your comments.
> > I am not able to understand the full purpose of the BitField functions.
> > Can you please help me to understand.
> >
> > As per my understanding :
> > BitFieldAnd32 (Operand, StartBit, EndBit, AndData) :
> >
> > We want to calculate the bitwise and (&) between Operand's bits (StartBit
> to EndBit, both inclusive) and AndData's bits (Startbit to Endbit, both
> inclusive).
> > If this is the case, then why is there a restriction on AndData ? (If
> > AndData is larger than the bitmask value range specified by StartBit
> > and EndBit, then ASSERT().)
> >
> > How these functions are intended to be used ?
> 
> StartBit and EndBit designate a bit-field within Operand. Operand is the
> "container" of the bit-field.
> 
> "AndData" is meant for the bit-field itself, not the container. AndData is
> applied relative to StartBit, and (relative to StartBit) it must not exceed
> EndBit.
> 
> So BitFieldAnd32() applies the AndData mask to the bit-field within the 32-bit
> container word. Bits outside of the bit-field are unchanged.
> 
> Laszlo
> 
> 
> >> -----Original Message-----
> >> From: Gao, Liming [mailto:liming.gao@intel.com]
> >> Sent: Friday, February 23, 2018 4:33 PM
> >> To: Pankaj Bansal <pankaj.bansal@nxp.com>; edk2-devel@lists.01.org
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> >> Subject: RE: [RFC] MdePkg/BaseLib: Change BitField functions.
> >>
> >> Pankaj:
> >>   OrData, AndData and Value are the bit field value specified by
> >> StartBit and EndBit. They are not the full value. They must be
> >> adjusted to the full value, then calculated with Operand.
> >>
> >>   And, we use LShit() or RShit() function for 64bit operand, because
> >> we find VS compiler may generate the intrinsic function when >> or <<
> >> is used 64bit operand on 32bit arch.
> >>
> >> Thanks
> >> Liming
> >>> -----Original Message-----
> >>> From: Pankaj Bansal [mailto:pankaj.bansal@nxp.com]
> >>> Sent: Tuesday, February 20, 2018 12:50 PM
> >>> To: edk2-devel@lists.01.org
> >>> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> >>> <michael.d.kinney@intel.com>
> >>> Subject: RE: [RFC] MdePkg/BaseLib: Change BitField functions.
> >>>
> >>> Hi everybody,
> >>>
> >>> Any comments on this change ?
> >>>
> >>>> -----Original Message-----
> >>>> From: Pankaj Bansal
> >>>> Sent: Friday, February 09, 2018 4:30 PM
> >>>> To: edk2-devel@lists.01.org
> >>>> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Michael D Kinney
> >>>> <michael.d.kinney@intel.com>; Liming Gao <liming.gao@intel.com>
> >>>> Subject: [RFC] MdePkg/BaseLib: Change BitField functions.
> >>>>
> >>>> The bit field functions in MdePkg are not working as expected.
> >>>> The restrictions on Value parameter such that Value should not be
> >>>> greater than the bitmask value range specified by StartBit and
> >>>> EndBit doesn't seem
> >>> to
> >>>> make sense.
> >>>>
> >>>> Also the restriction on End bit to not be equal to start bit
> >>>> prohibits single bit change.
> >>>>
> >>>> This is an attempt to correct these limitations.
> >>>>
> >>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >>>> Cc: Liming Gao <liming.gao@intel.com>
> >>>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>>> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> >>>> ---
> >>>>
> >>>> Notes:
> >>>>     This is a RFC patch.
> >>>>
> >>>>  MdePkg/Library/BaseLib/BitField.c | 179
> >>>> ++++++++++++++--------------
> >>>>  1 file changed, 89 insertions(+), 90 deletions(-)
> >>>>
> >>>> diff --git a/MdePkg/Library/BaseLib/BitField.c
> >>>> b/MdePkg/Library/BaseLib/BitField.c
> >>>> index eb9e276..2b5be52 100644
> >>>> --- a/MdePkg/Library/BaseLib/BitField.c
> >>>> +++ b/MdePkg/Library/BaseLib/BitField.c
> >>>> @@ -2,6 +2,8 @@
> >>>>    Bit field functions of BaseLib.
> >>>>
> >>>>    Copyright (c) 2006 - 2013, Intel Corporation. All rights
> >>>> reserved.<BR>
> >>>> +  Copyright 2018 NXP
> >>>> +
> >>>>    This program and the accompanying materials
> >>>>    are licensed and made available under the terms and conditions
> >>>> of the BSD License
> >>>>    which accompanies this distribution.  The full text of the
> >>>> license may be found at @@ -14,6 +16,12 @@
> >>>>
> >>>>  #include "BaseLibInternals.h"
> >>>>
> >>>> +#define BITMASK_UNITN(StartBit, EndBit) \
> >>>> +    (((MAX_UINTN) << (StartBit)) & (MAX_UINTN >> ((sizeof (UINTN)
> >>>> +*
> >>>> +8)
> >>>> +- 1 - (EndBit))))
> >>>> +
> >>>> +#define BITMASK_UNIT64(StartBit, EndBit) \
> >>>> +    (((MAX_UINT64) << (StartBit)) & (MAX_UINT64 >> ((sizeof
> >>>> +(UINT64)
> >>>> +*
> >>>> +8) - 1 - (EndBit))))
> >>>> +
> >>>>  /**
> >>>>    Worker function that returns a bit field from Operand.
> >>>>
> >>>> @@ -34,11 +42,9 @@ InternalBaseLibBitFieldReadUint (
> >>>>    IN      UINTN                     EndBit
> >>>>    )
> >>>>  {
> >>>> -  //
> >>>> -  // ~((UINTN)-2 << EndBit) is a mask in which bit[0] thru
> >>>> bit[EndBit]
> >>>> -  // are 1's while bit[EndBit + 1] thru the most significant bit are 0's.
> >>>> -  //
> >>>> -  return (Operand & ~((UINTN)-2 << EndBit)) >> StartBit;
> >>>> +  UINTN  Mask = BITMASK_UNITN (StartBit, EndBit);
> >>>> +
> >>>> +  return ( (Operand & Mask) >> StartBit);
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -68,19 +74,9 @@ InternalBaseLibBitFieldOrUint (
> >>>>    IN      UINTN                     OrData
> >>>>    )
> >>>>  {
> >>>> -  //
> >>>> -  // Higher bits in OrData those are not used must be zero.
> >>>> -  //
> >>>> -  // EndBit - StartBit + 1 might be 32 while the result right
> >>>> shifting 32 on a 32bit integer is undefined,
> >>>> -  // So the logic is updated to right shift (EndBit - StartBit)
> >>>> bits and compare the last bit directly.
> >>>> -  //
> >>>> -  ASSERT ((OrData >> (EndBit - StartBit)) == ((OrData >> (EndBit -
> >>>> StartBit)) & 1));
> >>>> -
> >>>> -  //
> >>>> -  // ~((UINTN)-2 << EndBit) is a mask in which bit[0] thru
> >>>> bit[EndBit]
> >>>> -  // are 1's while bit[EndBit + 1] thru the most significant bit are 0's.
> >>>> -  //
> >>>> -  return Operand | ((OrData << StartBit) & ~((UINTN) -2 <<
> >>>> EndBit));
> >>>> +  UINTN  Mask = BITMASK_UNITN (StartBit, EndBit);
> >>>> +
> >>>> +  return ( ( (Operand | OrData) & Mask) | (Operand & ~Mask));
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -110,19 +106,38 @@ InternalBaseLibBitFieldAndUint (
> >>>>    IN      UINTN                     AndData
> >>>>    )
> >>>>  {
> >>>> -  //
> >>>> -  // Higher bits in AndData those are not used must be zero.
> >>>> -  //
> >>>> -  // EndBit - StartBit + 1 might be 32 while the result right
> >>>> shifting 32 on a 32bit integer is undefined,
> >>>> -  // So the logic is updated to right shift (EndBit - StartBit)
> >>>> bits and compare the last bit directly.
> >>>> -  //
> >>>> -  ASSERT ((AndData >> (EndBit - StartBit)) == ((AndData >> (EndBit
> >>>> -
> >>> StartBit))
> >>>> & 1));
> >>>> -
> >>>> -  //
> >>>> -  // ~((UINTN)-2 << EndBit) is a mask in which bit[0] thru
> >>>> bit[EndBit]
> >>>> -  // are 1's while bit[EndBit + 1] thru the most significant bit are 0's.
> >>>> -  //
> >>>> -  return Operand & ~((~AndData << StartBit) & ~((UINTN)-2 <<
> >>>> EndBit));
> >>>> +  UINTN  Mask = BITMASK_UNITN (StartBit, EndBit);
> >>>> +
> >>>> +  return  ( ( (Operand & AndData) & Mask) | (Operand & ~Mask)); }
> >>>> +
> >>>> +/**
> >>>> +  Worker function that writes a bit field to an value, and returns
> >>>> +the
> >> result.
> >>>> +
> >>>> +  Writes Value to the bit field specified by the StartBit and the
> >>>> + EndBit in  Operand. All other bits in Operand are preserved. The
> >>>> + new 8-bit value is  returned.
> >>>> +
> >>>> +  @param  Operand   Operand on which to perform the bitfield
> >> operation.
> >>>> +  @param  StartBit  The ordinal of the least significant bit in the bit
> field.
> >>>> +  @param  EndBit    The ordinal of the most significant bit in the bit
> field.
> >>>> +  @param  Value     The new value of the bit field.
> >>>> +
> >>>> +  @return The new value.
> >>>> +
> >>>> +**/
> >>>> +UINTN
> >>>> +EFIAPI
> >>>> +InternalBaseLibBitFieldWriteUint (
> >>>> +  IN      UINTN                     Operand,
> >>>> +  IN      UINTN                     StartBit,
> >>>> +  IN      UINTN                     EndBit,
> >>>> +  IN      UINTN                     Value
> >>>> +  )
> >>>> +{
> >>>> +  UINTN  Mask = BITMASK_UNITN (StartBit, EndBit);
> >>>> +
> >>>> +  return  ( (Value & Mask) | (Operand & ~Mask));
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -153,7 +168,7 @@ BitFieldRead8 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 8);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT8)InternalBaseLibBitFieldReadUint (Operand,
> >>>> StartBit, EndBit);  }
> >>>>
> >>>> @@ -190,8 +205,8 @@ BitFieldWrite8 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 8);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> -  return BitFieldAndThenOr8 (Operand, StartBit, EndBit, 0, Value);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>> +  return (UINT8)InternalBaseLibBitFieldWriteUint (Operand,
> >>>> + StartBit, EndBit,
> >>>> Value);
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -228,7 +243,7 @@ BitFieldOr8 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 8);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT8)InternalBaseLibBitFieldOrUint (Operand, StartBit,
> >>>> EndBit, OrData);  }
> >>>>
> >>>> @@ -266,7 +281,7 @@ BitFieldAnd8 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 8);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT8)InternalBaseLibBitFieldAndUint (Operand, StartBit,
> >>>> EndBit, AndData);  }
> >>>>
> >>>> @@ -275,7 +290,7 @@ BitFieldAnd8 (
> >>>>    bitwise OR, and returns the result.
> >>>>
> >>>>    Performs a bitwise AND between the bit field specified by
> >>>> StartBit and EndBit
> >>>> -  in Operand and the value specified by AndData, followed by a
> >>>> bitwise
> >>>> +  in Operand and the value specified by AndData, followed by a
> >>>> + bitwise
> >>>>    OR with value specified by OrData. All other bits in Operand are
> >>>>    preserved. The new 8-bit value is returned.
> >>>>
> >>>> @@ -308,7 +323,7 @@ BitFieldAndThenOr8 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 8);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return BitFieldOr8 (
> >>>>             BitFieldAnd8 (Operand, StartBit, EndBit, AndData),
> >>>>             StartBit,
> >>>> @@ -345,7 +360,7 @@ BitFieldRead16 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 16);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT16)InternalBaseLibBitFieldReadUint (Operand,
> >>>> StartBit,
> >>> EndBit);
> >>>>  }
> >>>>
> >>>> @@ -382,8 +397,8 @@ BitFieldWrite16 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 16);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> -  return BitFieldAndThenOr16 (Operand, StartBit, EndBit, 0,
> >>>> Value);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>> +  return (UINT16)InternalBaseLibBitFieldWriteUint (Operand,
> >>>> + StartBit,
> >>>> EndBit, Value);
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -420,7 +435,7 @@ BitFieldOr16 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 16);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT16)InternalBaseLibBitFieldOrUint (Operand, StartBit,
> >>>> EndBit, OrData);  }
> >>>>
> >>>> @@ -458,7 +473,7 @@ BitFieldAnd16 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 16);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT16)InternalBaseLibBitFieldAndUint (Operand,
> >>>> StartBit, EndBit, AndData);  }
> >>>>
> >>>> @@ -467,7 +482,7 @@ BitFieldAnd16 (
> >>>>    bitwise OR, and returns the result.
> >>>>
> >>>>    Performs a bitwise AND between the bit field specified by
> >>>> StartBit and EndBit
> >>>> -  in Operand and the value specified by AndData, followed by a
> >>>> bitwise
> >>>> +  in Operand and the value specified by AndData, followed by a
> >>>> + bitwise
> >>>>    OR with value specified by OrData. All other bits in Operand are
> >>>>    preserved. The new 16-bit value is returned.
> >>>>
> >>>> @@ -500,7 +515,7 @@ BitFieldAndThenOr16 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 16);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return BitFieldOr16 (
> >>>>             BitFieldAnd16 (Operand, StartBit, EndBit, AndData),
> >>>>             StartBit,
> >>>> @@ -537,7 +552,7 @@ BitFieldRead32 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 32);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT32)InternalBaseLibBitFieldReadUint (Operand,
> >>>> StartBit,
> >>> EndBit);
> >>>>  }
> >>>>
> >>>> @@ -574,8 +589,8 @@ BitFieldWrite32 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 32);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> -  return BitFieldAndThenOr32 (Operand, StartBit, EndBit, 0,
> >>>> Value);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>> +  return (UINT32)InternalBaseLibBitFieldWriteUint (Operand,
> >>>> + StartBit,
> >>>> EndBit, Value);
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -612,7 +627,7 @@ BitFieldOr32 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 32);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT32)InternalBaseLibBitFieldOrUint (Operand, StartBit,
> >>>> EndBit, OrData);  }
> >>>>
> >>>> @@ -650,7 +665,7 @@ BitFieldAnd32 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 32);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return (UINT32)InternalBaseLibBitFieldAndUint (Operand,
> >>>> StartBit, EndBit, AndData);  }
> >>>>
> >>>> @@ -659,7 +674,7 @@ BitFieldAnd32 (
> >>>>    bitwise OR, and returns the result.
> >>>>
> >>>>    Performs a bitwise AND between the bit field specified by
> >>>> StartBit and EndBit
> >>>> -  in Operand and the value specified by AndData, followed by a
> >>>> bitwise
> >>>> +  in Operand and the value specified by AndData, followed by a
> >>>> + bitwise
> >>>>    OR with value specified by OrData. All other bits in Operand are
> >>>>    preserved. The new 32-bit value is returned.
> >>>>
> >>>> @@ -692,7 +707,7 @@ BitFieldAndThenOr32 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 32);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return BitFieldOr32 (
> >>>>             BitFieldAnd32 (Operand, StartBit, EndBit, AndData),
> >>>>             StartBit,
> >>>> @@ -729,8 +744,11 @@ BitFieldRead64 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 64);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> -  return RShiftU64 (Operand & ~LShiftU64 ((UINT64)-2, EndBit),
> >>>> StartBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>> +
> >>>> +  UINT64  Mask = BITMASK_UNIT64 (StartBit, EndBit);
> >>>> +
> >>>> +  return ( (Operand & Mask) >> StartBit);
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -766,8 +784,11 @@ BitFieldWrite64 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 64);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> -  return BitFieldAndThenOr64 (Operand, StartBit, EndBit, 0,
> >>>> Value);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>> +
> >>>> +  UINT64  Mask = BITMASK_UNIT64 (StartBit, EndBit);
> >>>> +
> >>>> +  return  ( (Value & Mask) | (Operand & ~Mask));
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -803,23 +824,12 @@ BitFieldOr64 (
> >>>>    IN      UINT64                    OrData
> >>>>    )
> >>>>  {
> >>>> -  UINT64  Value1;
> >>>> -  UINT64  Value2;
> >>>> -
> >>>>    ASSERT (EndBit < 64);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> -  //
> >>>> -  // Higher bits in OrData those are not used must be zero.
> >>>> -  //
> >>>> -  // EndBit - StartBit + 1 might be 64 while the result right
> >>>> shifting 64 on
> >>>> RShiftU64() API is invalid,
> >>>> -  // So the logic is updated to right shift (EndBit - StartBit)
> >>>> bits and compare the last bit directly.
> >>>> -  //
> >>>> -  ASSERT (RShiftU64 (OrData, EndBit - StartBit) == (RShiftU64
> >>>> (OrData,
> >>> EndBit
> >>>> - StartBit) & 1));
> >>>> -
> >>>> -  Value1 = LShiftU64 (OrData, StartBit);
> >>>> -  Value2 = LShiftU64 ((UINT64) - 2, EndBit);
> >>>> -
> >>>> -  return Operand | (Value1 & ~Value2);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>> +
> >>>> +  UINT64  Mask = BITMASK_UNIT64 (StartBit, EndBit);
> >>>> +
> >>>> +  return ( ( (Operand | OrData) & Mask) | (Operand & ~Mask));
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -855,23 +865,12 @@ BitFieldAnd64 (
> >>>>    IN      UINT64                    AndData
> >>>>    )
> >>>>  {
> >>>> -  UINT64  Value1;
> >>>> -  UINT64  Value2;
> >>>> -
> >>>>    ASSERT (EndBit < 64);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> -  //
> >>>> -  // Higher bits in AndData those are not used must be zero.
> >>>> -  //
> >>>> -  // EndBit - StartBit + 1 might be 64 while the right shifting 64
> >>>> on RShiftU64() API is invalid,
> >>>> -  // So the logic is updated to right shift (EndBit - StartBit)
> >>>> bits and compare the last bit directly.
> >>>> -  //
> >>>> -  ASSERT (RShiftU64 (AndData, EndBit - StartBit) == (RShiftU64
> >>>> (AndData, EndBit - StartBit) & 1));
> >>>> -
> >>>> -  Value1 = LShiftU64 (~AndData, StartBit);
> >>>> -  Value2 = LShiftU64 ((UINT64)-2, EndBit);
> >>>> -
> >>>> -  return Operand & ~(Value1 & ~Value2);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>> +
> >>>> +  UINT64  Mask = BITMASK_UNIT64 (StartBit, EndBit);
> >>>> +
> >>>> +  return  ( ( (Operand & AndData) & Mask) | (Operand & ~Mask));
> >>>>  }
> >>>>
> >>>>  /**
> >>>> @@ -879,7 +878,7 @@ BitFieldAnd64 (
> >>>>    bitwise OR, and returns the result.
> >>>>
> >>>>    Performs a bitwise AND between the bit field specified by
> >>>> StartBit and EndBit
> >>>> -  in Operand and the value specified by AndData, followed by a
> >>>> bitwise
> >>>> +  in Operand and the value specified by AndData, followed by a
> >>>> + bitwise
> >>>>    OR with value specified by OrData. All other bits in Operand are
> >>>>    preserved. The new 64-bit value is returned.
> >>>>
> >>>> @@ -912,7 +911,7 @@ BitFieldAndThenOr64 (
> >>>>    )
> >>>>  {
> >>>>    ASSERT (EndBit < 64);
> >>>> -  ASSERT (StartBit <= EndBit);
> >>>> +  ASSERT (StartBit < EndBit);
> >>>>    return BitFieldOr64 (
> >>>>             BitFieldAnd64 (Operand, StartBit, EndBit, AndData),
> >>>>             StartBit,
> >>>> --
> >>>> 2.7.4
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cpankaj.bans
> >
> al%40nxp.com%7Ced613e79935445554a2608d57ab6c6cd%7C686ea1d3bc2b
> 4c6fa92c
> >
> d99c5c301635%7C0%7C0%7C636549847796367463&sdata=RyFpJ4kcJnCDCOS
> D103AWe
> > 8cOycTiDBRBYlCFsF9cCE%3D&reserved=0
> >


      reply	other threads:[~2018-02-26  6:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-09 10:59 [RFC] MdePkg/BaseLib: Change BitField functions Pankaj Bansal
2018-02-20  4:49 ` Pankaj Bansal
2018-02-23 11:02   ` Gao, Liming
2018-02-23 11:16     ` Pankaj Bansal
2018-02-23 12:12       ` Laszlo Ersek
2018-02-26  6:33         ` Pankaj Bansal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR0402MB3940128A7CB425D548CC1B04F1C10@AM0PR0402MB3940.eurprd04.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox